[Attributor] Allow PHI nodes in AAValueConstantRangeFloating

Traversing PHI nodes is natural with the genericValueTraversal but also
a bit tricky. The problem is similar to the ones we have seen in AAAlign
and AADereferenceable, namely that we continue to increase the range in
each iteration. We use a pessimistic approach here to stop the
iterations. Nevertheless, optimistic information can now be propagated
through a PHI node.

diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index da5260b..91eb561 100644 (file)
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -6187,9 +6187,9 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
       if (CI->getOperand(0)->getType()->isIntegerTy())
         return;
 
-    // We can work with select instruction as we traverse their operands
+    // We can work with PHI and select instruction as we traverse their operands
     // during update.
-    if (isa<SelectInst>(V))
+    if (isa<SelectInst>(V) || isa<PHINode>(V))
       return;
 
     // Otherwise we give up.
@@ -6199,17 +6199,21 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
                       << getAssociatedValue() << "\n");
   }
 
-  bool calculateBinaryOperator(Attributor &A, BinaryOperator *BinOp,
-                               IntegerRangeState &T, Instruction *CtxI) {
+  bool calculateBinaryOperator(
+      Attributor &A, BinaryOperator *BinOp, IntegerRangeState &T,
+      Instruction *CtxI,
+      SmallVectorImpl<const AAValueConstantRange *> &QuerriedAAs) {
     Value *LHS = BinOp->getOperand(0);
     Value *RHS = BinOp->getOperand(1);
 
     auto &LHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*LHS));
+    QuerriedAAs.push_back(&LHSAA);
     auto LHSAARange = LHSAA.getAssumedConstantRange(A, CtxI);
 
     auto &RHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*RHS));
+    QuerriedAAs.push_back(&RHSAA);
     auto RHSAARange = RHSAA.getAssumedConstantRange(A, CtxI);
 
     auto AssumedRange = LHSAARange.binaryOp(BinOp->getOpcode(), RHSAARange);
@@ -6221,8 +6225,9 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
     return T.isValidState();
   }
 
-  bool calculateCastInst(Attributor &A, CastInst *CastI, IntegerRangeState &T,
-                         Instruction *CtxI) {
+  bool calculateCastInst(
+      Attributor &A, CastInst *CastI, IntegerRangeState &T, Instruction *CtxI,
+      SmallVectorImpl<const AAValueConstantRange *> &QuerriedAAs) {
     assert(CastI->getNumOperands() == 1 && "Expected cast to be unary!");
     // TODO: Allow non integers as well.
     Value &OpV = *CastI->getOperand(0);
@@ -6230,20 +6235,25 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
 
     auto &OpAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(OpV));
+    QuerriedAAs.push_back(&OpAA);
     T.unionAssumed(
         OpAA.getAssumed().castOp(CastI->getOpcode(), getState().getBitWidth()));
     return T.isValidState();
   }
 
-  bool calculateCmpInst(Attributor &A, CmpInst *CmpI, IntegerRangeState &T,
-                        Instruction *CtxI) {
+  bool
+  calculateCmpInst(Attributor &A, CmpInst *CmpI, IntegerRangeState &T,
+                   Instruction *CtxI,
+                   SmallVectorImpl<const AAValueConstantRange *> &QuerriedAAs) {
     Value *LHS = CmpI->getOperand(0);
     Value *RHS = CmpI->getOperand(1);
 
     auto &LHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*LHS));
+    QuerriedAAs.push_back(&LHSAA);
     auto &RHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*RHS));
+    QuerriedAAs.push_back(&RHSAA);
 
     auto LHSAARange = LHSAA.getAssumedConstantRange(A, CtxI);
     auto RHSAARange = RHSAA.getAssumedConstantRange(A, CtxI);
@@ -6301,19 +6311,37 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
         return T.isValidState();
       }
 
-      if (auto *BinOp = dyn_cast<BinaryOperator>(I))
-        return calculateBinaryOperator(A, BinOp, T, CtxI);
-      else if (auto *CmpI = dyn_cast<CmpInst>(I))
-        return calculateCmpInst(A, CmpI, T, CtxI);
-      else if (auto *CastI = dyn_cast<CastInst>(I))
-        return calculateCastInst(A, CastI, T, CtxI);
-      else {
+      SmallVector<const AAValueConstantRange *, 4> QuerriedAAs;
+      if (auto *BinOp = dyn_cast<BinaryOperator>(I)) {
+        if (!calculateBinaryOperator(A, BinOp, T, CtxI, QuerriedAAs))
+          return false;
+      } else if (auto *CmpI = dyn_cast<CmpInst>(I)) {
+        if (!calculateCmpInst(A, CmpI, T, CtxI, QuerriedAAs))
+          return false;
+      } else if (auto *CastI = dyn_cast<CastInst>(I)) {
+        if (!calculateCastInst(A, CastI, T, CtxI, QuerriedAAs))
+          return false;
+      } else {
         // Give up with other instructions.
         // TODO: Add other instructions
 
         T.indicatePessimisticFixpoint();
         return false;
       }
+
+      // Catch circular reasoning in a pessimistic way for now.
+      // TODO: Check how the range evolves and if we stripped anything, see also
+      //       AADereferenceable or AAAlign for similar situations.
+      for (const AAValueConstantRange *QueriedAA : QuerriedAAs) {
+        if (QueriedAA != this)
+          continue;
+        // If we are in a stady state we do not need to worry.
+        if (T.getAssumed() == getState().getAssumed())
+          continue;
+        T.indicatePessimisticFixpoint();
+      }
+
+      return T.isValidState();
     };
 
     IntegerRangeState T(getBitWidth());

diff --git a/llvm/test/Transforms/Attributor/liveness.ll b/llvm/test/Transforms/Attributor/liveness.ll
index 37b10ed..d8e879e 100644 (file)
--- a/llvm/test/Transforms/Attributor/liveness.ll
+++ b/llvm/test/Transforms/Attributor/liveness.ll
@@ -1,8 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes
-; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,MODULE,ALL_BUT_OLD_CGSCCC
-; RUN: opt -attributor-cgscc --attributor-disable=false -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
-; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,MODULE,ALL_BUT_OLD_CGSCCC
-; RUN: opt -passes='attributor-cgscc' --attributor-disable=false -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC,ALL_BUT_OLD_CGSCCC
+; RUN: opt -attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,MODULE,ALL_BUT_OLD_CGSCCC
+; RUN: opt -attributor-cgscc --attributor-disable=false -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
+; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,MODULE,ALL_BUT_OLD_CGSCCC
+; RUN: opt -passes='attributor-cgscc' --attributor-disable=false -attributor-annotate-decl-cs -attributor-max-iterations=6 -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC,ALL_BUT_OLD_CGSCCC
 ; UTC_ARGS: --turn off
 
 ; ALL_BUT_OLD_CGSCCC: @dead_with_blockaddress_users.l = constant [2 x i8*] [i8* inttoptr (i32 1 to i8*), i8* inttoptr (i32 1 to i8*)]

diff --git a/llvm/test/Transforms/Attributor/range.ll b/llvm/test/Transforms/Attributor/range.ll
index 97d5425..812345e 100644 (file)
--- a/llvm/test/Transforms/Attributor/range.ll
+++ b/llvm/test/Transforms/Attributor/range.ll
@@ -543,61 +543,30 @@ declare dso_local i32 @foo(i32)
 
 ; FIXME: All but the return is not needed anymore
 define dso_local zeroext i1 @phi(i32 %arg) {
-; OLD_PM-LABEL: define {{[^@]+}}@phi
-; OLD_PM-SAME: (i32 [[ARG:%.*]])
-; OLD_PM-NEXT:  bb:
-; OLD_PM-NEXT:    [[TMP:%.*]] = icmp sgt i32 [[ARG]], 5
-; OLD_PM-NEXT:    br i1 [[TMP]], label [[BB1:%.*]], label [[BB2:%.*]]
-; OLD_PM:       bb1:
-; OLD_PM-NEXT:    br label [[BB3:%.*]]
-; OLD_PM:       bb2:
-; OLD_PM-NEXT:    br label [[BB3]]
-; OLD_PM:       bb3:
-; OLD_PM-NEXT:    [[DOT02:%.*]] = phi i32 [ 1, [[BB1]] ], [ 2, [[BB2]] ]
-; OLD_PM-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[ARG]], 10
-; OLD_PM-NEXT:    br i1 [[TMP4]], label [[BB5:%.*]], label [[BB7:%.*]]
-; OLD_PM:       bb5:
-; OLD_PM-NEXT:    [[TMP6:%.*]] = add nsw i32 [[DOT02]], 1
-; OLD_PM-NEXT:    br label [[BB9:%.*]]
-; OLD_PM:       bb7:
-; OLD_PM-NEXT:    [[TMP8:%.*]] = add nsw i32 [[DOT02]], 2
-; OLD_PM-NEXT:    br label [[BB9]]
-; OLD_PM:       bb9:
-; OLD_PM-NEXT:    [[DOT01:%.*]] = phi i32 [ [[TMP6]], [[BB5]] ], [ [[TMP8]], [[BB7]] ]
-; OLD_PM-NEXT:    [[TMP10:%.*]] = icmp eq i32 [[DOT01]], 5
-; OLD_PM-NEXT:    br i1 [[TMP10]], label [[BB11:%.*]], label [[BB12:%.*]]
-; OLD_PM:       bb11:
-; OLD_PM-NEXT:    br label [[BB13:%.*]]
-; OLD_PM:       bb12:
-; OLD_PM-NEXT:    br label [[BB13]]
-; OLD_PM:       bb13:
-; OLD_PM-NEXT:    [[DOT0:%.*]] = phi i1 [ true, [[BB11]] ], [ false, [[BB12]] ]
-; OLD_PM-NEXT:    ret i1 [[DOT0]]
-;
-; NEW_PM-LABEL: define {{[^@]+}}@phi
-; NEW_PM-SAME: (i32 [[ARG:%.*]])
-; NEW_PM-NEXT:  bb:
-; NEW_PM-NEXT:    [[TMP:%.*]] = icmp sgt i32 [[ARG]], 5
-; NEW_PM-NEXT:    br i1 [[TMP]], label [[BB1:%.*]], label [[BB2:%.*]]
-; NEW_PM:       bb1:
-; NEW_PM-NEXT:    br label [[BB3:%.*]]
-; NEW_PM:       bb2:
-; NEW_PM-NEXT:    br label [[BB3]]
-; NEW_PM:       bb3:
-; NEW_PM-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[ARG]], 10
-; NEW_PM-NEXT:    br i1 [[TMP4]], label [[BB5:%.*]], label [[BB7:%.*]]
-; NEW_PM:       bb5:
-; NEW_PM-NEXT:    br label [[BB9:%.*]]
-; NEW_PM:       bb7:
-; NEW_PM-NEXT:    br label [[BB9]]
-; NEW_PM:       bb9:
-; NEW_PM-NEXT:    br label [[BB12:%.*]]
-; NEW_PM:       bb11:
-; NEW_PM-NEXT:    unreachable
-; NEW_PM:       bb12:
-; NEW_PM-NEXT:    br label [[BB13:%.*]]
-; NEW_PM:       bb13:
-; NEW_PM-NEXT:    ret i1 false
+; CHECK-LABEL: define {{[^@]+}}@phi
+; CHECK-SAME: (i32 [[ARG:%.*]])
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP:%.*]] = icmp sgt i32 [[ARG]], 5
+; CHECK-NEXT:    br i1 [[TMP]], label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    br label [[BB3]]
+; CHECK:       bb3:
+; CHECK-NEXT:    [[TMP4:%.*]] = icmp sgt i32 [[ARG]], 10
+; CHECK-NEXT:    br i1 [[TMP4]], label [[BB5:%.*]], label [[BB7:%.*]]
+; CHECK:       bb5:
+; CHECK-NEXT:    br label [[BB9:%.*]]
+; CHECK:       bb7:
+; CHECK-NEXT:    br label [[BB9]]
+; CHECK:       bb9:
+; CHECK-NEXT:    br label [[BB12:%.*]]
+; CHECK:       bb11:
+; CHECK-NEXT:    unreachable
+; CHECK:       bb12:
+; CHECK-NEXT:    br label [[BB13:%.*]]
+; CHECK:       bb13:
+; CHECK-NEXT:    ret i1 false
 ;
 bb:
   %tmp = icmp sgt i32 %arg, 5

diff --git a/llvm/test/Transforms/Attributor/willreturn.ll b/llvm/test/Transforms/Attributor/willreturn.ll
index c366dee..12b79a4 100644 (file)
--- a/llvm/test/Transforms/Attributor/willreturn.ll
+++ b/llvm/test/Transforms/Attributor/willreturn.ll
@@ -1,4 +1,4 @@
-; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,ATTRIBUTOR_MODULE
+; RUN: opt -passes=attributor --attributor-disable=false -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=7 -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,ATTRIBUTOR_MODULE
 ; RUN: opt -passes=attributor-cgscc --attributor-disable=false -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,ATTRIBUTOR_CGSCC