[IPSCCP] Intersect attribute info for interprocedural args

[nikic]

IPSCCP can currently return worse results than SCCP for arguments that are tracked interprocedurally, because information from attributes is not used for them.

Fix this by intersecting in the attribute information when propagating lattice values from calls.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-function-specialization

Author: Nikita Popov (nikic)

Changes

IPSCCP can currently return worse results than SCCP for arguments that are tracked interprocedurally, because information from attributes is not used for them.

Fix this by intersecting in the attribute information when propagating lattice values from calls.

---

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

3 Files Affected:

• (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+15-11)
• (modified) llvm/test/Transforms/SCCP/pointer-nonnull.ll (+10-6)
• (modified) llvm/test/Transforms/SCCP/range-attribute.ll (+16-11)

diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 982b1041c7c514..59775d2199ca61 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -820,19 +820,21 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> {
       markOverdefined(ValueState[V], V);
   }
 
-  void trackValueOfArgument(Argument *A) {
+  ValueLatticeElement getArgAttributeVL(Argument *A) {
     if (A->getType()->isIntOrIntVectorTy()) {
-      if (std::optional<ConstantRange> Range = A->getRange()) {
-        markConstantRange(ValueState[A], A, *Range);
-        return;
-      }
-    }
-    if (A->hasNonNullAttr()) {
-      markNotNull(ValueState[A], A);
-      return;
+      if (std::optional<ConstantRange> Range = A->getRange())
+        return ValueLatticeElement::getRange(*Range);
     }
+    if (A->hasNonNullAttr())
+      return ValueLatticeElement::getNot(Constant::getNullValue(A->getType()));
     // Assume nothing about the incoming arguments without attributes.
-    markOverdefined(A);
+    return ValueLatticeElement::getOverdefined();
+  }
+
+  void trackValueOfArgument(Argument *A) {
+    if (A->getType()->isStructTy())
+      return (void)markOverdefined(A);
+    mergeInValue(A, getArgAttributeVL(A));
   }
 
   bool isStructLatticeConstant(Function *F, StructType *STy);
@@ -1800,7 +1802,9 @@ void SCCPInstVisitor::handleCallArguments(CallBase &CB) {
                        getMaxWidenStepsOpts());
         }
       } else
-        mergeInValue(&*AI, getValueState(*CAI), getMaxWidenStepsOpts());
+        mergeInValue(&*AI,
+                     getValueState(*CAI).intersect(getArgAttributeVL(&*AI)),
+                     getMaxWidenStepsOpts());
     }
   }
 }
diff --git a/llvm/test/Transforms/SCCP/pointer-nonnull.ll b/llvm/test/Transforms/SCCP/pointer-nonnull.ll
index d035df399b77be..54eb12317680e4 100644
--- a/llvm/test/Transforms/SCCP/pointer-nonnull.ll
+++ b/llvm/test/Transforms/SCCP/pointer-nonnull.ll
@@ -210,18 +210,22 @@ define internal i1 @ip_test_nonnull_callee(ptr nonnull %p) {
 ;
 ; IPSCCP-LABEL: define internal i1 @ip_test_nonnull_callee(
 ; IPSCCP-SAME: ptr nonnull [[P:%.*]]) {
-; IPSCCP-NEXT:    [[CMP:%.*]] = icmp ne ptr [[P]], null
-; IPSCCP-NEXT:    ret i1 [[CMP]]
+; IPSCCP-NEXT:    ret i1 poison
 ;
   %cmp = icmp ne ptr %p, null
   ret i1 %cmp
 }
 
 define i1 @ip_test_nonnull_caller(ptr %p) {
-; CHECK-LABEL: define i1 @ip_test_nonnull_caller(
-; CHECK-SAME: ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[RES:%.*]] = call i1 @ip_test_nonnull_callee(ptr [[P]])
-; CHECK-NEXT:    ret i1 [[RES]]
+; SCCP-LABEL: define i1 @ip_test_nonnull_caller(
+; SCCP-SAME: ptr [[P:%.*]]) {
+; SCCP-NEXT:    [[RES:%.*]] = call i1 @ip_test_nonnull_callee(ptr [[P]])
+; SCCP-NEXT:    ret i1 [[RES]]
+;
+; IPSCCP-LABEL: define i1 @ip_test_nonnull_caller(
+; IPSCCP-SAME: ptr [[P:%.*]]) {
+; IPSCCP-NEXT:    [[RES:%.*]] = call i1 @ip_test_nonnull_callee(ptr [[P]])
+; IPSCCP-NEXT:    ret i1 true
 ;
   %res = call i1 @ip_test_nonnull_callee(ptr %p)
   ret i1 %res
diff --git a/llvm/test/Transforms/SCCP/range-attribute.ll b/llvm/test/Transforms/SCCP/range-attribute.ll
index c55eb03a5c8158..207732bf2bac4d 100644
--- a/llvm/test/Transforms/SCCP/range-attribute.ll
+++ b/llvm/test/Transforms/SCCP/range-attribute.ll
@@ -193,8 +193,7 @@ define i1 @ip_range_attribute_constant() {
 
 define internal i1 @ip_cmp_attribute_overdefined_callee(i32 range(i32 0, 10) %x) {
 ; IPSCCP-LABEL: @ip_cmp_attribute_overdefined_callee(
-; IPSCCP-NEXT:    [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 10
-; IPSCCP-NEXT:    ret i1 [[CMP]]
+; IPSCCP-NEXT:    ret i1 poison
 ;
 ; SCCP-LABEL: @ip_cmp_attribute_overdefined_callee(
 ; SCCP-NEXT:    ret i1 true
@@ -204,9 +203,13 @@ define internal i1 @ip_cmp_attribute_overdefined_callee(i32 range(i32 0, 10) %x)
 }
 
 define i1 @ip_cmp_attribute_overdefined_caller(i32 %x) {
-; CHECK-LABEL: @ip_cmp_attribute_overdefined_caller(
-; CHECK-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_overdefined_callee(i32 [[X:%.*]])
-; CHECK-NEXT:    ret i1 [[RES]]
+; IPSCCP-LABEL: @ip_cmp_attribute_overdefined_caller(
+; IPSCCP-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_overdefined_callee(i32 [[X:%.*]])
+; IPSCCP-NEXT:    ret i1 true
+;
+; SCCP-LABEL: @ip_cmp_attribute_overdefined_caller(
+; SCCP-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_overdefined_callee(i32 [[X:%.*]])
+; SCCP-NEXT:    ret i1 [[RES]]
 ;
   %res = call i1 @ip_cmp_attribute_overdefined_callee(i32 %x)
   ret i1 %res
@@ -214,9 +217,7 @@ define i1 @ip_cmp_attribute_overdefined_caller(i32 %x) {
 
 define internal i1 @ip_cmp_attribute_intersect_callee(i32 range(i32 0, 10) %x) {
 ; IPSCCP-LABEL: @ip_cmp_attribute_intersect_callee(
-; IPSCCP-NEXT:    [[CMP1:%.*]] = icmp ult i32 [[X:%.*]], 10
-; IPSCCP-NEXT:    [[AND:%.*]] = and i1 [[CMP1]], true
-; IPSCCP-NEXT:    ret i1 [[AND]]
+; IPSCCP-NEXT:    ret i1 poison
 ;
 ; SCCP-LABEL: @ip_cmp_attribute_intersect_callee(
 ; SCCP-NEXT:    [[CMP2:%.*]] = icmp uge i32 [[X:%.*]], 5
@@ -230,9 +231,13 @@ define internal i1 @ip_cmp_attribute_intersect_callee(i32 range(i32 0, 10) %x) {
 }
 
 define i1 @ip_cmp_attribute_intersect_caller(i32 range(i32 5, 15) %x) {
-; CHECK-LABEL: @ip_cmp_attribute_intersect_caller(
-; CHECK-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_intersect_callee(i32 [[X:%.*]])
-; CHECK-NEXT:    ret i1 [[RES]]
+; IPSCCP-LABEL: @ip_cmp_attribute_intersect_caller(
+; IPSCCP-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_intersect_callee(i32 [[X:%.*]])
+; IPSCCP-NEXT:    ret i1 true
+;
+; SCCP-LABEL: @ip_cmp_attribute_intersect_caller(
+; SCCP-NEXT:    [[RES:%.*]] = call i1 @ip_cmp_attribute_intersect_callee(i32 [[X:%.*]])
+; SCCP-NEXT:    ret i1 [[RES]]
 ;
   %res = call i1 @ip_cmp_attribute_intersect_callee(i32 %x)
   ret i1 %res

[fhahn]

LTM, thanks!

[dtcxzyw]

Is it correct? Shouldn't it be true?

[nikic]

This is because IPSCCP replaces the the use of the return value in the caller, so the return value in the callee becomes unused and is replaced with poison.

[fhahn]

For IPSCCP, returned value isn't used any longer (at call sites we replaced by true)

[dtcxzyw]

LG