[NewGVN] Use PredicateInfo info when previously used for the same ssa.copy intrinsic

Symbolic execution using PredicateInfo is only done for the ssa.copy
intrinsic. It's using two potential sources for building the expression:
1. the Value of the instruction for which the instruction is a copy of, and
2. the Value from the contraint in PredicateInfo
It's possible to get into an infinite loop when choosing between these
two, as described in PR31613.

This patch proposes performing swapping of the two values (i.e. choosing
the second one for the expression), if that same second value was chosen
before; this breaks the cycle.

In the testcases provided, where there is a contradiction between the
value from symbolic execution and assume instruction, NewGVN reduces the
assume to assume(false).

Resolves PR31613.

Differential Revision: https://reviews.llvm.org/D110907

llvm/lib/Transforms/Scalar/NewGVN.cpp

@@ -638,6 +638,7 @@ class NewGVN {
   BitVector TouchedInstructions;
 
   DenseMap<const BasicBlock *, std::pair<unsigned, unsigned>> BlockInstRange;
+  mutable DenseMap<const IntrinsicInst *, const Value *> IntrinsicInstPred;
 
 #ifndef NDEBUG
   // Debugging for how many times each block and instruction got processed.
@@ -794,7 +795,7 @@ private:
                                                  BasicBlock *PHIBlock) const;
   const Expression *performSymbolicAggrValueEvaluation(Instruction *) const;
   ExprResult performSymbolicCmpEvaluation(Instruction *) const;
-  ExprResult performSymbolicPredicateInfoEvaluation(Instruction *) const;
+  ExprResult performSymbolicPredicateInfoEvaluation(IntrinsicInst *) const;
 
   // Congruence finding.
   bool someEquivalentDominates(const Instruction *, const Instruction *) const;
@@ -815,6 +816,8 @@ private:
   // Ranking
   unsigned int getRank(const Value *) const;
   bool shouldSwapOperands(const Value *, const Value *) const;
+  bool shouldSwapOperandsForIntrinsic(const Value *, const Value *,
+                                      const IntrinsicInst *I) const;
 
   // Reachability handling.
   void updateReachableEdge(BasicBlock *, BasicBlock *);
@@ -1552,7 +1555,7 @@ const Expression *NewGVN::performSymbolicLoadEvaluation(Instruction *I) const {
 }
 
 NewGVN::ExprResult
-NewGVN::performSymbolicPredicateInfoEvaluation(Instruction *I) const {
+NewGVN::performSymbolicPredicateInfoEvaluation(IntrinsicInst *I) const {
   auto *PI = PredInfo->getPredicateInfoFor(I);
   if (!PI)
     return ExprResult::none();
@@ -1572,7 +1575,7 @@ NewGVN::performSymbolicPredicateInfoEvaluation(Instruction *I) const {
   Value *AdditionallyUsedValue = CmpOp0;
 
   // Sort the ops.
-  if (shouldSwapOperands(FirstOp, SecondOp)) {
+  if (shouldSwapOperandsForIntrinsic(FirstOp, SecondOp, I)) {
     std::swap(FirstOp, SecondOp);
     Predicate = CmpInst::getSwappedPredicate(Predicate);
     AdditionallyUsedValue = CmpOp1;
@@ -1598,7 +1601,7 @@ NewGVN::ExprResult NewGVN::performSymbolicCallEvaluation(Instruction *I) const {
     // Intrinsics with the returned attribute are copies of arguments.
     if (auto *ReturnedValue = II->getReturnedArgOperand()) {
       if (II->getIntrinsicID() == Intrinsic::ssa_copy)
-        if (auto Res = performSymbolicPredicateInfoEvaluation(I))
+        if (auto Res = performSymbolicPredicateInfoEvaluation(II))
           return Res;
       return ExprResult::some(createVariableOrConstant(ReturnedValue));
     }
@@ -2951,6 +2954,7 @@ void NewGVN::cleanupTables() {
   PredicateToUsers.clear();
   MemoryToUsers.clear();
   RevisitOnReachabilityChange.clear();
+  IntrinsicInstPred.clear();
 }
 
 // Assign local DFS number mapping to instructions, and leave space for Value
@@ -4152,6 +4156,29 @@ bool NewGVN::shouldSwapOperands(const Value *A, const Value *B) const {
   return std::make_pair(getRank(A), A) > std::make_pair(getRank(B), B);
 }
 
+bool NewGVN::shouldSwapOperandsForIntrinsic(const Value *A, const Value *B,
+                                            const IntrinsicInst *I) const {
+  auto LookupResult = IntrinsicInstPred.find(I);
+  if (shouldSwapOperands(A, B)) {
+    if (LookupResult == IntrinsicInstPred.end())
+      IntrinsicInstPred.insert({I, B});
+    else
+      LookupResult->second = B;
+    return true;
+  }
+
+  if (LookupResult != IntrinsicInstPred.end()) {
+    auto *SeenPredicate = LookupResult->second;
+    if (SeenPredicate) {
+      if (SeenPredicate == B)
+        return true;
+      else
+        LookupResult->second = nullptr;
+    }
+  }
+  return false;
+}
+
 namespace {
 
 class NewGVNLegacyPass : public FunctionPass {

llvm/test/Transforms/NewGVN/pr31613_2.ll

@@ -0,0 +1,141 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=newgvn -S | FileCheck %s
+; REQUIRES: asserts
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-grtev4-linux-gnu"
+
+define hidden void @barrier() align 2 {
+; CHECK-LABEL: @barrier(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[CALLG:%.*]] = tail call i64 @g()
+; CHECK-NEXT:    [[SEL:%.*]] = select i1 undef, i64 0, i64 [[CALLG]]
+; CHECK-NEXT:    [[LOADED:%.*]] = load i64, i64* null, align 8
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[LOADED]], 1
+; CHECK-NEXT:    [[SHR17:%.*]] = lshr i64 [[ADD]], 1
+; CHECK-NEXT:    [[SUB:%.*]] = add nsw i64 [[SHR17]], -1
+; CHECK-NEXT:    br label [[FIRST:%.*]]
+; CHECK:       first:
+; CHECK-NEXT:    [[PHI_ONE:%.*]] = phi i64 [ [[SEL]], [[ENTRY:%.*]] ], [ 0, [[FIRST]] ], [ 0, [[THIRD:%.*]] ]
+; CHECK-NEXT:    [[CMP_PHI1_SUB:%.*]] = icmp eq i64 [[PHI_ONE]], [[SUB]]
+; CHECK-NEXT:    br i1 [[CMP_PHI1_SUB]], label [[SECOND:%.*]], label [[FIRST]]
+; CHECK:       second:
+; CHECK-NEXT:    br label [[THIRD]]
+; CHECK:       third:
+; CHECK-NEXT:    br i1 false, label [[SECOND]], label [[FIRST]]
+;
+entry:
+  %callg = tail call i64 @g()
+  %sel = select i1 undef, i64 0, i64 %callg
+
+  %loaded = load i64, i64* null, align 8
+  %add = add i64 %loaded, 1
+  %shr17 = lshr i64 %add, 1
+  %sub = add nsw i64 %shr17, -1
+
+  br label %first
+
+first:
+  %phi_one = phi i64 [ %sel, %entry ], [ 0, %first ], [ 0, %third ]
+  %cmp_phi1_sub = icmp eq i64 %phi_one, %sub
+  br i1 %cmp_phi1_sub, label %second, label %first
+
+second:
+  %phi_two = phi i64 [ %inc, %third ], [ %phi_one, %first ]
+  br label %third
+
+third:
+  %inc = add i64 %phi_two, 1
+  %cmp_inc_sub = icmp eq i64 %inc, %sub
+  br i1 %cmp_inc_sub, label %second, label %first
+}
+
+define hidden void @barrier2() align 2 {
+; CHECK-LABEL: @barrier2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i64, i64* null, align 8
+; CHECK-NEXT:    [[CALL9:%.*]] = tail call i64 @g()
+; CHECK-NEXT:    [[REM:%.*]] = select i1 undef, i64 0, i64 [[CALL9]]
+; CHECK-NEXT:    [[ADD:%.*]] = add i64 [[TMP0]], 1
+; CHECK-NEXT:    [[SHR17:%.*]] = lshr i64 [[ADD]], 1
+; CHECK-NEXT:    [[SUB:%.*]] = add nsw i64 [[SHR17]], -1
+; CHECK-NEXT:    br label [[MAINLOOP:%.*]]
+; CHECK:       second.exit:
+; CHECK-NEXT:    br label [[FIRST_EXIT:%.*]]
+; CHECK:       first.exit:
+; CHECK-NEXT:    br label [[MAINLOOP]]
+; CHECK:       mainloop:
+; CHECK-NEXT:    [[FIRSTPHI:%.*]] = phi i64 [ [[REM]], [[ENTRY:%.*]] ], [ 0, [[FIRST_EXIT]] ]
+; CHECK-NEXT:    [[FIRSTCMP:%.*]] = icmp eq i64 [[FIRSTPHI]], [[SUB]]
+; CHECK-NEXT:    br i1 [[FIRSTCMP]], label [[SECOND_PREHEADER:%.*]], label [[FIRST_EXIT]]
+; CHECK:       second.preheader:
+; CHECK-NEXT:    br label [[INNERLOOP:%.*]]
+; CHECK:       innerloop:
+; CHECK-NEXT:    br label [[CLEANUP:%.*]]
+; CHECK:       cleanup:
+; CHECK-NEXT:    br i1 false, label [[INNERLOOP]], label [[SECOND_EXIT:%.*]]
+;
+entry:
+  %0 = load i64, i64* null, align 8
+  %call9 = tail call i64 @g()
+  %rem = select i1 undef, i64 0, i64 %call9
+  %add = add i64 %0, 1
+  %shr17 = lshr i64 %add, 1
+  %sub = add nsw i64 %shr17, -1
+  br label %mainloop
+
+second.exit:                       ; preds = %cleanup
+  br label %first.exit
+
+first.exit:                                ; preds = %mainloop, %second.exit
+  br label %mainloop
+
+mainloop:                                         ; preds = %first.exit, %entry
+  %firstphi = phi i64 [ %rem, %entry ], [ 0, %first.exit ]
+  %firstcmp = icmp eq i64 %firstphi, %sub
+  br i1 %firstcmp, label %second.preheader, label %first.exit
+
+second.preheader:                          ; preds = %mainloop
+  br label %innerloop
+
+innerloop:                                    ; preds = %cleanup, %second.preheader
+  %secondphi = phi i64 [ %inc, %cleanup ], [ %firstphi, %second.preheader ]
+  br label %cleanup
+
+cleanup:                                     ; preds = %innerloop
+  %inc = add i64 %secondphi, 1
+  %secondcmp = icmp eq i64 %inc, %sub
+  br i1 %secondcmp, label %innerloop, label %second.exit
+}
+
+declare hidden i64 @g() local_unnamed_addr align 2
+
+define void @barrier3(i64 %arg) {
+; CHECK-LABEL: @barrier3(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FIRSTLOOP:%.*]]
+; CHECK:       firstloop:
+; CHECK-NEXT:    [[PHI1:%.*]] = phi i64 [ [[ARG:%.*]], [[ENTRY:%.*]] ], [ 0, [[FIRSTLOOP]] ]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i64 [[PHI1]], -1
+; CHECK-NEXT:    br i1 [[CMP1]], label [[SECONDLOOP:%.*]], label [[FIRSTLOOP]]
+; CHECK:       secondloop:
+; CHECK-NEXT:    call void @llvm.assume(i1 false)
+; CHECK-NEXT:    br label [[SECONDLOOP]]
+;
+entry:
+  br label %firstloop
+
+firstloop:
+  %phi1 = phi i64 [ %arg, %entry ], [ 0, %firstloop ]
+  %cmp1 = icmp eq i64 %phi1, -1
+  br i1 %cmp1, label %secondloop, label %firstloop
+
+secondloop:
+  %phi2 = phi i64 [ %inc, %secondloop ], [ %phi1, %firstloop ]
+  %inc = add i64 %phi2, 1
+  %cmp2 = icmp eq i64 %inc, -1
+  call void @llvm.assume(i1 %cmp2)
+  br label %secondloop
+}
+
+declare void @llvm.assume(i1)