Skip to content
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.

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

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

Merged
merged 2 commits into from
Apr 13, 2024
Merged

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Apr 12, 2024

This patch uses getConstantRangeAtUse to infer nsw/nuw flags with at-use info. It will enables more optimizations in InstCombine.

Compile-time impact: http://llvm-compile-time-tracker.com/compare.php?from=a5ed14bc8e122fa5ac0aa81f8d8390931bd6b4e4&to=a83d3402b663439b91cb37a046fb7ac0220ba5c7&stat=instructions%3Au

Related issue: #87854

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 12, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch uses getConstantRangeAtUse to infer nsw/nuw flags with at-use info. It will enables more optimizations in InstCombine.

Compile-time impact: http://llvm-compile-time-tracker.com/compare.php?from=a5ed14bc8e122fa5ac0aa81f8d8390931bd6b4e4&to=a83d3402b663439b91cb37a046fb7ac0220ba5c7&stat=instructions%3Au

Related issue: #87854


Full diff: https://github.com/llvm/llvm-project/pull/88523.diff

3 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+4-7)
  • (modified) llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll (+34)
  • (modified) llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll (+2-1)
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index fb334e8292eb18..715cdaff972796 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -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;
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll b/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll
index 0b95139f3dcbaa..3af4c70a5621c8 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/cond-at-use.ll
@@ -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
+; 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) {
+; 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
+}
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
index 7f102bc1e4fe2e..7e712947e37ad9 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
@@ -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:

; 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.

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

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Apr 12, 2024
@goldsteinn
Copy link
Contributor

LGTM. Wait on another.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@dtcxzyw dtcxzyw merged commit df9c00b into llvm:main Apr 13, 2024
6 checks passed
@dtcxzyw dtcxzyw deleted the perf/cvp-obo-at-use branch April 13, 2024 07:07
@XChy
Copy link
Member

XChy commented Apr 13, 2024

Thanks. Looks like we have replaced all getConstantRange with getConstantRangeAtUse in CVP.

bazuzi pushed a commit to bazuzi/llvm-project that referenced this pull request Apr 15, 2024
This patch uses `getConstantRangeAtUse` to infer nsw/nuw flags with
at-use info. It will enables more optimizations in InstCombine.

Compile-time impact:
http://llvm-compile-time-tracker.com/compare.php?from=a5ed14bc8e122fa5ac0aa81f8d8390931bd6b4e4&to=a83d3402b663439b91cb37a046fb7ac0220ba5c7&stat=instructions%3Au

Related issue: llvm#87854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants