[llvm] r312510 - NewGVN: Fix PR 34430 - we need to look through predicateinfo copies to detect self-cycles of phi nodes. We also need to not ignore certain types of arguments when testing whether the phi has a backedge or was originally constant.

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 4 19:17:43 PDT 2017


Author: dannyb
Date: Mon Sep  4 19:17:43 2017
New Revision: 312510

URL: http://llvm.org/viewvc/llvm-project?rev=312510&view=rev
Log:
NewGVN: Fix PR 34430 - we need to look through predicateinfo copies to detect self-cycles of phi nodes.  We also need to not ignore certain types of arguments when testing whether the phi has a backedge or was originally constant.

Added:
    llvm/trunk/test/Transforms/NewGVN/pr34430.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=312510&r1=312509&r2=312510&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Mon Sep  4 19:17:43 2017
@@ -851,14 +851,22 @@ void NewGVN::deleteExpression(const Expr
   ExpressionAllocator.Deallocate(E);
 }
 
-// Return true if V is really PN, even accounting for predicateinfo copies.
-static bool isCopyOfSelf(const Value *V, const PHINode *PN) {
-  if (V == PN)
-    return V;
+// If V is a predicateinfo copy, get the thing it is a copy of.
+static Value *getCopyOf(const Value *V) {
   if (auto *II = dyn_cast<IntrinsicInst>(V))
-    if (II->getIntrinsicID() == Intrinsic::ssa_copy && II->getOperand(0) == PN)
-      return true;
-  return false;
+    if (II->getIntrinsicID() == Intrinsic::ssa_copy)
+      return II->getOperand(0);
+  return nullptr;
+}
+
+// Return true if V is really PN, even accounting for predicateinfo copies.
+static bool isCopyOfPHI(const Value *V, const PHINode *PN) {
+  return V == PN || getCopyOf(V) == PN;
+}
+
+static bool isCopyOfAPHI(const Value *V) {
+  auto *CO = getCopyOf(V);
+  return CO && isa<PHINode>(CO);
 }
 
 PHIExpression *NewGVN::createPHIExpression(Instruction *I, bool &HasBackedge,
@@ -890,23 +898,21 @@ PHIExpression *NewGVN::createPHIExpressi
 
   // Filter out unreachable phi operands.
   auto Filtered = make_filter_range(PHIOperands, [&](const Use *U) {
-    if (isCopyOfSelf(*U, PN))
+    auto *BB = PN->getIncomingBlock(*U);
+    if (isCopyOfPHI(*U, PN))
       return false;
-    if (!ReachableEdges.count({PN->getIncomingBlock(*U), PHIBlock}))
+    if (!ReachableEdges.count({BB, PHIBlock}))
       return false;
     // Things in TOPClass are equivalent to everything.
     if (ValueToClass.lookup(*U) == TOPClass)
       return false;
+    OriginalOpsConstant = OriginalOpsConstant && isa<Constant>(*U);
+    HasBackedge = HasBackedge || isBackedge(BB, PHIBlock);
     return lookupOperandLeader(*U) != PN;
   });
-  std::transform(Filtered.begin(), Filtered.end(), op_inserter(E),
-                 [&](const Use *U) -> Value * {
-                   auto *BB = PN->getIncomingBlock(*U);
-                   HasBackedge = HasBackedge || isBackedge(BB, PHIBlock);
-                   OriginalOpsConstant =
-                       OriginalOpsConstant && isa<Constant>(*U);
-                   return lookupOperandLeader(*U);
-                 });
+  std::transform(
+      Filtered.begin(), Filtered.end(), op_inserter(E),
+      [&](const Use *U) -> Value * { return lookupOperandLeader(*U); });
   return E;
 }
 
@@ -990,7 +996,6 @@ const Expression *NewGVN::checkSimplific
         addAdditionalUsers(V, I);
       return createVariableOrConstant(CC->getLeader());
     }
-
     if (CC->getDefiningExpr()) {
       // If we simplified to something else, we need to communicate
       // that we're users of the value we simplified to.
@@ -1611,8 +1616,9 @@ bool NewGVN::isCycleFree(const Instructi
     if (SCC.size() == 1)
       InstCycleState.insert({I, ICS_CycleFree});
     else {
-      bool AllPhis =
-          llvm::all_of(SCC, [](const Value *V) { return isa<PHINode>(V); });
+      bool AllPhis = llvm::all_of(SCC, [](const Value *V) {
+        return isa<PHINode>(V) || isCopyOfAPHI(V);
+      });
       ICS = AllPhis ? ICS_CycleFree : ICS_Cycle;
       for (auto *Member : SCC)
         if (auto *MemberPhi = dyn_cast<PHINode>(Member))
@@ -1632,9 +1638,9 @@ const Expression *NewGVN::performSymboli
   // This is really shorthand for "this phi cannot cycle due to forward
   // change in value of the phi is guaranteed not to later change the value of
   // the phi. IE it can't be v = phi(undef, v+1)
-  bool AllConstant = true;
-  auto *E =
-      cast<PHIExpression>(createPHIExpression(I, HasBackedge, AllConstant));
+  bool OriginalOpsConstant = true;
+  auto *E = cast<PHIExpression>(
+      createPHIExpression(I, HasBackedge, OriginalOpsConstant));
   // We match the semantics of SimplifyPhiNode from InstructionSimplify here.
   // See if all arguments are the same.
   // We track if any were undef because they need special handling.
@@ -1660,14 +1666,10 @@ const Expression *NewGVN::performSymboli
     deleteExpression(E);
     return createDeadExpression();
   }
-  unsigned NumOps = 0;
   Value *AllSameValue = *(Filtered.begin());
   ++Filtered.begin();
   // Can't use std::equal here, sadly, because filter.begin moves.
-  if (llvm::all_of(Filtered, [&](Value *Arg) {
-        ++NumOps;
-        return Arg == AllSameValue;
-      })) {
+  if (llvm::all_of(Filtered, [&](Value *Arg) { return Arg == AllSameValue; })) {
     // In LLVM's non-standard representation of phi nodes, it's possible to have
     // phi nodes with cycles (IE dependent on other phis that are .... dependent
     // on the original phi node), especially in weird CFG's where some arguments
@@ -1682,9 +1684,8 @@ const Expression *NewGVN::performSymboli
       // multivalued phi, and we need to know if it's cycle free in order to
       // evaluate whether we can ignore the undef.  The other parts of this are
       // just shortcuts.  If there is no backedge, or all operands are
-      // constants, or all operands are ignored but the undef, it also must be
-      // cycle free.
-      if (!AllConstant && HasBackedge && NumOps > 0 &&
+      // constants, it also must be cycle free.
+      if (HasBackedge && !OriginalOpsConstant &&
           !isa<UndefValue>(AllSameValue) && !isCycleFree(I))
         return E;
 

Added: llvm/trunk/test/Transforms/NewGVN/pr34430.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/pr34430.ll?rev=312510&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/pr34430.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/pr34430.ll Mon Sep  4 19:17:43 2017
@@ -0,0 +1,48 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; ModuleID = 'bugpoint-reduced-simplified.bc'
+; RUN: opt < %s -newgvn -S | FileCheck %s
+source_filename = "bugpoint-output-e4c7d0f.bc"
+
+;  Make sure we still properly resolve phi cycles when they involve predicateinfo copies of phis.
+define void @hoge() local_unnamed_addr #0 {
+; CHECK-LABEL: @hoge(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    br i1 undef, label [[BB6:%.*]], label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br label [[BB6]]
+; CHECK:       bb2:
+; CHECK-NEXT:    br i1 true, label [[BB3:%.*]], label [[BB6]]
+; CHECK:       bb3:
+; CHECK-NEXT:    br label [[BB4:%.*]]
+; CHECK:       bb4:
+; CHECK-NEXT:    br i1 undef, label [[BB2:%.*]], label [[BB6]]
+; CHECK:       bb6:
+; CHECK-NEXT:    br label [[BB4]]
+;
+bb:
+  br i1 undef, label %bb6, label %bb1
+
+bb1:                                              ; preds = %bb
+  br label %bb6
+
+bb2:                                              ; preds = %bb4
+  %tmp = icmp slt i8 %tmp5, 7
+  br i1 %tmp, label %bb3, label %bb6
+
+bb3:                                              ; preds = %bb2
+  br label %bb4
+
+bb4:                                              ; preds = %bb6, %bb3
+  %tmp5 = phi i8 [ %tmp5, %bb3 ], [ %tmp7, %bb6 ]
+  br i1 undef, label %bb2, label %bb6
+
+bb6:                                              ; preds = %bb4, %bb2, %bb1, %bb
+  %tmp7 = phi i8 [ %tmp5, %bb4 ], [ %tmp5, %bb2 ], [ 5, %bb1 ], [ undef, %bb ]
+  br label %bb4
+}
+
+attributes #0 = { norecurse noreturn nounwind ssp uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="penryn" "target-features"="+cx16,+fxsr,+mmx,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.ident = !{!0}
+
+!0 = !{!"clang version 6.0.0 (http://llvm.org/git/clang.git e649d902285b23af8ba58cb92a739f3bad2723df) (/Users/dannyb/sources/llvm-clean 0abfd30028cbb294ff2c2dd5e2df4ec3fdb6c591)"}




More information about the llvm-commits mailing list