Skip to content

[CVP] Use at-use info in processBinOp #88523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 13, 2024
Merged

[CVP] Use at-use info in processBinOp #88523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
691f19d
[CVP] Add pre-commit tests. NFC.
dtcxzyw Apr 12, 2024
a83d340
[CVP] Use at-use info in `processBinOp`
dtcxzyw Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,13 +1105,10 @@ static bool processBinOp(BinaryOperator *BinOp, LazyValueInfo *LVI) {
return false;

Instruction::BinaryOps Opcode = BinOp->getOpcode();
Value *LHS = BinOp->getOperand(0);
Value *RHS = BinOp->getOperand(1);

ConstantRange LRange =
LVI->getConstantRange(LHS, BinOp, /*UndefAllowed*/ false);
ConstantRange RRange =
LVI->getConstantRange(RHS, BinOp, /*UndefAllowed*/ false);
ConstantRange LRange = LVI->getConstantRangeAtUse(BinOp->getOperandUse(0),
/*UndefAllowed=*/false);
ConstantRange RRange = LVI->getConstantRangeAtUse(BinOp->getOperandUse(1),
/*UndefAllowed=*/false);

bool Changed = false;
bool NewNUW = false, NewNSW = false;
Expand Down
34 changes: 34 additions & 0 deletions llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,3 +596,37 @@ define i16 @and_elide_poison_flags_missing_noundef(i16 %a) {
%sel = select i1 %cmp, i16 %and, i16 24
ret i16 %sel
}

define i32 @pr87854(i32 noundef %x.1, i32 noundef %i) {
; CHECK-LABEL: @pr87854(
; CHECK-NEXT: [[COND:%.*]] = icmp sgt i32 [[X_1:%.*]], -1
; CHECK-NEXT: tail call void @llvm.assume(i1 [[COND]])
; CHECK-NEXT: [[INBOUNDS:%.*]] = icmp ult i32 [[I:%.*]], [[X_1]]
; CHECK-NEXT: [[NEXT:%.*]] = add nuw i32 [[I]], 1
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why the nsw flag cannot be inferred from the assumption. intersectAssumeOrGuardBlockValueConstantRange should handle this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think intersectAssumeOrGuardBlockValueConstantRange should be the one getting the nsw, its the select that enables it.
but either way, I think the issue is that UseBlockValue is false so we don't actually compute the %x.1 CR when evaluating the inbounds cmp.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, we may want to test increasing const unsigned MaxUsesToInspect = 3;. Seems startlingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue is that UseBlockValue is false so we don't actually compute the %x.1 CR when evaluating the inbounds cmp.

Yes, we don't use block values when deriving information from select conditions right now. I'm not sure doing this is viable, especially in the at-use logic.

; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[INBOUNDS]], i32 [[NEXT]], i32 -1
; CHECK-NEXT: ret i32 [[SPEC_SELECT]]
;
%cond = icmp sgt i32 %x.1, -1
tail call void @llvm.assume(i1 %cond)
%inbounds = icmp ult i32 %i, %x.1
%next = add i32 %i, 1
%spec.select = select i1 %inbounds, i32 %next, i32 -1
ret i32 %spec.select
}

define i64 @test_shl_nsw_at_use(i64 noundef %x) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be folded by instcombine later: https://godbolt.org/z/4MGsvnsnT

; CHECK-LABEL: @test_shl_nsw_at_use(
; CHECK-NEXT: [[ADD:%.*]] = add i64 [[X:%.*]], 2147483648
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i64 [[ADD]], 4294967296
; CHECK-NEXT: [[SHL:%.*]] = shl nsw i64 [[X]], 32
; CHECK-NEXT: [[SHR:%.*]] = ashr exact i64 [[SHL]], 32
; CHECK-NEXT: [[RES:%.*]] = select i1 [[CMP]], i64 [[SHR]], i64 0
; CHECK-NEXT: ret i64 [[RES]]
;
%add = add i64 %x, 2147483648
%cmp = icmp ult i64 %add, 4294967296
%shl = shl i64 %x, 32
%shr = ashr exact i64 %shl, 32
%res = select i1 %cmp, i64 %shr, i64 0
ret i64 %res
}
3 changes: 2 additions & 1 deletion llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,13 @@ bb3:
define i32 @PR43802_without_nowrap(i32 %arg) {
; CHECK-LABEL: @PR43802_without_nowrap(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[SUB:%.*]] = sub i32 0, [[ARG:%.*]]
; CHECK-NEXT: [[SUB1:%.*]] = sub nsw i32 0, [[ARG:%.*]]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[ARG]], -2147483648
; CHECK-NEXT: br i1 [[CMP]], label [[BB2:%.*]], label [[BB3:%.*]]
; CHECK: bb2:
; CHECK-NEXT: br label [[BB3]]
; CHECK: bb3:
; CHECK-NEXT: [[SUB:%.*]] = phi i32 [ -2147483648, [[BB2]] ], [ [[SUB1]], [[ENTRY:%.*]] ]
; CHECK-NEXT: ret i32 [[SUB]]
;
entry:
Expand Down
Loading