[llvm] r314612 - NewGVN: Fix PR 34473, by not using ExactlyEqualsExpression for finding

[Daniel Berlin via llvm-commits]

Author: dannyb
Date: Sat Sep 30 16:51:55 2017
New Revision: 314612

URL: http://llvm.org/viewvc/llvm-project?rev=314612&view=rev
Log:
NewGVN: Fix PR 34473, by not using ExactlyEqualsExpression for finding
phi of ops users.

Modified:
    llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
    llvm/trunk/test/Transforms/NewGVN/completeness.ll

Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=314612&r1=314611&r2=314612&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Sat Sep 30 16:51:55 2017
@@ -2249,7 +2249,7 @@ void NewGVN::moveValueToNewCongruenceCla
 // For a given expression, mark the phi of ops instructions that could have
 // changed as a result.
 void NewGVN::markPhiOfOpsChanged(const Expression *E) {
-  touchAndErase(ExpressionToPhiOfOps, ExactEqualsExpression(*E));
+  touchAndErase(ExpressionToPhiOfOps, E);
 }
 
 // Perform congruence finding on a given value numbering expression.
@@ -2668,6 +2668,7 @@ NewGVN::makePossiblePHIOfOps(Instruction
     if (!DebugCounter::shouldExecute(PHIOfOpsCounter))
       return nullptr;
     SmallVector<ValPair, 4> Ops;
+    SmallPtrSet<Value *, 4> Deps;
     auto *PHIBlock = getBlockForValue(OpPHI);
     RevisitOnReachabilityChange[PHIBlock].reset(InstrToDFSNum(I));
     for (unsigned PredNum = 0; PredNum < OpPHI->getNumOperands(); ++PredNum) {
@@ -2686,18 +2687,15 @@ NewGVN::makePossiblePHIOfOps(Instruction
         for (auto &Op : ValueOp->operands()) {
           auto *OrigOp = &*Op;
           // When these operand changes, it could change whether there is a
-          // leader for us or not, so we have to add additional users
+          // leader for us or not, so we have to add additional users.
           if (isa<PHINode>(Op)) {
             Op = Op->DoPHITranslation(PHIBlock, PredBB);
             if (Op != OrigOp && Op != I)
-              addAdditionalUsers(Op, I);
+              Deps.insert(Op);
           } else if (auto *ValuePHI = RealToTemp.lookup(Op)) {
             if (getBlockForValue(ValuePHI) == PHIBlock)
               Op = ValuePHI->getIncomingValue(PredNum);
           }
-          // When this operand changes, it could change whether there is a
-          // leader for us or not.
-          addAdditionalUsers(Op, I);
           // If we phi-translated the op, it must be safe.
           SafeForPHIOfOps = SafeForPHIOfOps &&
                             (Op != OrigOp ||
@@ -2726,6 +2724,8 @@ NewGVN::makePossiblePHIOfOps(Instruction
       DEBUG(dbgs() << "Found phi of ops operand " << *FoundVal << " in "
                    << getBlockName(PredBB) << "\n");
     }
+    for (auto Dep : Deps)
+      addAdditionalUsers(Dep, I);
     sortPHIOps(Ops);
     auto *E = performSymbolicPHIEvaluation(Ops, I, PHIBlock);
     if (isa<ConstantExpression>(E) || isa<VariableExpression>(E)) {

Modified: llvm/trunk/test/Transforms/NewGVN/completeness.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/completeness.ll?rev=314612&r1=314611&r2=314612&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/completeness.ll (original)
+++ llvm/trunk/test/Transforms/NewGVN/completeness.ll Sat Sep 30 16:51:55 2017
@@ -545,3 +545,61 @@ bb7:
 bb8:                                              ; preds = %bb
   ret void
 }
+
+;; Make sure we reprocess phi of ops involving loads when loads change class.
+;; This is PR 34473
+define void @test13() {
+; CHECK-LABEL: @test13(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[TMP:%.*]] = load i8, i8* null
+; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:       bb3:
+; CHECK-NEXT:    [[PHIOFOPS:%.*]] = phi i8 [ [[TMP]], [[BB1]] ], [ [[TMP10:%.*]], [[BB3]] ]
+; CHECK-NEXT:    [[TMP4:%.*]] = phi i8* [ null, [[BB1]] ], [ [[TMP6:%.*]], [[BB3]] ]
+; CHECK-NEXT:    [[TMP5:%.*]] = phi i32 [ undef, [[BB1]] ], [ [[TMP9:%.*]], [[BB3]] ]
+; CHECK-NEXT:    [[TMP6]] = getelementptr i8, i8* [[TMP4]], i64 1
+; CHECK-NEXT:    [[TMP8:%.*]] = sext i8 [[PHIOFOPS]] to i32
+; CHECK-NEXT:    [[TMP9]] = mul i32 [[TMP5]], [[TMP8]]
+; CHECK-NEXT:    [[TMP10]] = load i8, i8* [[TMP6]]
+; CHECK-NEXT:    [[TMP11:%.*]] = icmp eq i8 [[TMP10]], 0
+; CHECK-NEXT:    br i1 [[TMP11]], label [[BB12:%.*]], label [[BB3]]
+; CHECK:       bb12:
+; CHECK-NEXT:    [[TMP14:%.*]] = icmp eq i32 [[TMP9]], 0
+; CHECK-NEXT:    br i1 [[TMP14]], label [[BB1]], label [[BB15:%.*]]
+; CHECK:       bb15:
+; CHECK-NEXT:    call void (...) @bar()
+; CHECK-NEXT:    br label [[BB1]]
+;
+bb:
+  br label %bb1
+
+bb1:                                              ; preds = %bb15, %bb12, %bb
+  %tmp = load i8, i8* null
+  %tmp2 = icmp eq i8 %tmp, 8
+  br label %bb3
+
+bb3:                                              ; preds = %bb3, %bb1
+  %tmp4 = phi i8* [ null, %bb1 ], [ %tmp6, %bb3 ]
+  %tmp5 = phi i32 [ undef, %bb1 ], [ %tmp9, %bb3 ]
+  %tmp6 = getelementptr i8, i8* %tmp4, i64 1
+  %tmp7 = load i8, i8* %tmp4
+  %tmp8 = sext i8 %tmp7 to i32
+  %tmp9 = mul i32 %tmp5, %tmp8
+  %tmp10 = load i8, i8* %tmp6
+  %tmp11 = icmp eq i8 %tmp10, 0
+  br i1 %tmp11, label %bb12, label %bb3
+
+bb12:                                             ; preds = %bb3
+  %tmp13 = phi i32 [ %tmp9, %bb3 ]
+  %tmp14 = icmp eq i32 %tmp13, 0
+  br i1 %tmp14, label %bb1, label %bb15
+
+bb15:                                             ; preds = %bb12
+  call void (...) @bar()
+  br label %bb1
+}
+
+declare void @bar(...)
+