[llvm] r330334 - [NewGVN] Add ops as dependency if we cannot find a leader for ValueOp.

[Florian Hahn via llvm-commits]

Author: fhahn
Date: Thu Apr 19 08:05:47 2018
New Revision: 330334

URL: http://llvm.org/viewvc/llvm-project?rev=330334&view=rev
Log:
[NewGVN] Add ops as dependency if we cannot find a leader for ValueOp.

If those operands change, we might find a leader for ValueOp, which
could enable new phi-of-op creation.

This fixes a case where we missed creating a phi-of-ops node. With D43865
and this patch, bootstrapping clang/llvm works with -enable-newgvn, whereas
without it, the "value changed after iteration" assertion is triggered.

Reviewers: dberlin, davide

Reviewed By: dberlin

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

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

Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=330334&r1=330333&r2=330334&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Thu Apr 19 08:05:47 2018
@@ -2753,6 +2753,7 @@ NewGVN::makePossiblePHIOfOps(Instruction
         // Clone the instruction, create an expression from it that is
         // translated back into the predecessor, and see if we have a leader.
         Instruction *ValueOp = I->clone();
+        SmallPtrSet<Value *, 4> CurrentDeps;
         if (MemAccess)
           TempToMemory.insert({ValueOp, MemAccess});
         bool SafeForPHIOfOps = true;
@@ -2764,7 +2765,7 @@ NewGVN::makePossiblePHIOfOps(Instruction
           if (isa<PHINode>(Op)) {
             Op = Op->DoPHITranslation(PHIBlock, PredBB);
             if (Op != OrigOp && Op != I)
-              Deps.insert(Op);
+              CurrentDeps.insert(Op);
           } else if (auto *ValuePHI = RealToTemp.lookup(Op)) {
             if (getBlockForValue(ValuePHI) == PHIBlock)
               Op = ValuePHI->getIncomingValueForBlock(PredBB);
@@ -2783,8 +2784,16 @@ NewGVN::makePossiblePHIOfOps(Instruction
                                     : findLeaderForInst(ValueOp, Visited,
                                                         MemAccess, I, PredBB);
         ValueOp->deleteValue();
-        if (!FoundVal)
+        if (!FoundVal) {
+          // We failed to find a leader for the current ValueOp, but this might
+          // change in case of the translated operands change.
+          if (SafeForPHIOfOps)
+            for (auto Dep : CurrentDeps)
+              addAdditionalUsers(Dep, I);
+
           return nullptr;
+        }
+        Deps.insert(CurrentDeps.begin(), CurrentDeps.end());
       } else {
         DEBUG(dbgs() << "Skipping phi of ops operand for incoming block "
                      << getBlockName(PredBB)

Added: llvm/trunk/test/Transforms/NewGVN/pr35074.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/pr35074.ll?rev=330334&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/pr35074.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/pr35074.ll Thu Apr 19 08:05:47 2018
@@ -0,0 +1,62 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -newgvn -S | FileCheck %s
+
+define void @sort(i64 %.16) {
+; CHECK-LABEL: @sort(
+; CHECK-NEXT:  Entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = extractvalue { i64, i1 } undef, 0
+; CHECK-NEXT:    [[TMP1:%.*]] = lshr i64 [[TMP0]], 2
+; CHECK-NEXT:    br i1 undef, label [[DIVZEROFAIL2_I:%.*]], label [[WHILEBODY_LR_PH:%.*]]
+; CHECK:       DivZeroFail2.i:
+; CHECK-NEXT:    unreachable
+; CHECK:       WhileBody.lr.ph:
+; CHECK-NEXT:    [[TMP2:%.*]] = udiv i64 [[DOT16:%.*]], [[TMP1]]
+; CHECK-NEXT:    br label [[WHILEBODY:%.*]]
+; CHECK:       WhileBody:
+; CHECK-NEXT:    [[ITERATOR:%.*]] = phi i64 [ 0, [[WHILEBODY_LR_PH]] ], [ [[TMP6:%.*]], [[BOUNDSCHECKOK276:%.*]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 [[ITERATOR]], i64 [[TMP2]])
+; CHECK-NEXT:    [[TMP4:%.*]] = extractvalue { i64, i1 } [[TMP3]], 0
+; CHECK-NEXT:    [[TMP5:%.*]] = tail call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 [[TMP4]], i64 1)
+; CHECK-NEXT:    [[TMP6]] = extractvalue { i64, i1 } [[TMP5]], 0
+; CHECK-NEXT:    br i1 false, label [[BOUNDSCHECKFAIL275:%.*]], label [[BOUNDSCHECKOK276]]
+; CHECK:       WhileEnd:
+; CHECK-NEXT:    ret void
+; CHECK:       BoundsCheckFail275:
+; CHECK-NEXT:    unreachable
+; CHECK:       BoundsCheckOk276:
+; CHECK-NEXT:    [[TMP7:%.*]] = icmp ult i64 [[TMP6]], [[DOT16]]
+; CHECK-NEXT:    br i1 [[TMP7]], label [[WHILEBODY]], label [[WHILEEND:%.*]]
+;
+Entry:
+  %0 = extractvalue { i64, i1 } undef, 0
+  %1 = lshr i64 %0, 2
+  br i1 undef, label %DivZeroFail2.i, label %WhileBody.lr.ph
+
+DivZeroFail2.i:                                   ; preds = %Entry
+  unreachable
+
+WhileBody.lr.ph:                                  ; preds = %Entry
+  %2 = udiv i64 %.16, %1
+  br label %WhileBody
+
+WhileBody:                                        ; preds = %BoundsCheckOk276, %WhileBody.lr.ph
+  %iterator = phi i64 [ 0, %WhileBody.lr.ph ], [ %6, %BoundsCheckOk276 ]
+  %3 = tail call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %iterator, i64 %2)
+  %4 = extractvalue { i64, i1 } %3, 0
+  %5 = tail call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %4, i64 1)
+  %6 = extractvalue { i64, i1 } %5, 0
+  %7 = icmp ugt i64 %iterator, %.16
+  br i1 %7, label %BoundsCheckFail275, label %BoundsCheckOk276
+
+WhileEnd:                                         ; preds = %BoundsCheckOk276
+  ret void
+
+BoundsCheckFail275:                               ; preds = %WhileBody
+  unreachable
+
+BoundsCheckOk276:                                 ; preds = %WhileBody
+  %8 = icmp ult i64 %6, %.16
+  br i1 %8, label %WhileBody, label %WhileEnd
+}
+
+declare { i64, i1 } @llvm.uadd.with.overflow.i64(i64, i64)