[llvm-branch-commits] [llvm] bfd2c21 - [IR][LoopRotate] avoid leaving phi with no operands (PR48296)

[Sanjay Patel via llvm-branch-commits]

Author: Sanjay Patel
Date: 2020-11-30T09:28:45-05:00
New Revision: bfd2c216ea8ef09f8fb1f755ca2b89f86f74acbb

URL: https://github.com/llvm/llvm-project/commit/bfd2c216ea8ef09f8fb1f755ca2b89f86f74acbb
DIFF: https://github.com/llvm/llvm-project/commit/bfd2c216ea8ef09f8fb1f755ca2b89f86f74acbb.diff

LOG: [IR][LoopRotate] avoid leaving phi with no operands (PR48296)

https://llvm.org/PR48296 shows an example where we delete all of the operands
of a phi without actually deleting the phi, and that is currently considered
invalid IR. The reduced test included here would crash for that reason.

A suggested follow-up is to loosen the assert to allow 0-operand phis
in unreachable blocks.

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

Added: 
    llvm/test/Transforms/LoopRotate/phi-empty.ll

Modified: 
    llvm/include/llvm/IR/BasicBlock.h
    llvm/lib/IR/BasicBlock.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 26cfdd9e51d6..149b0a26c1f3 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -387,9 +387,9 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   /// Update PHI nodes in this BasicBlock before removal of predecessor \p Pred.
   /// Note that this function does not actually remove the predecessor.
   ///
-  /// If \p KeepOneInputPHIs is true then don't remove PHIs that are left with
-  /// zero or one incoming values, and don't simplify PHIs with all incoming
-  /// values the same.
+  /// If \p KeepOneInputPHIs is true, then don't remove PHIs that are left with
+  /// one incoming value and don't simplify PHIs with all incoming values the
+  /// same.
   void removePredecessor(BasicBlock *Pred, bool KeepOneInputPHIs = false);
 
   bool canSplitPredecessors() const;

diff  --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 3268641ddf19..aee769aa0fea 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -330,7 +330,7 @@ void BasicBlock::removePredecessor(BasicBlock *Pred,
 
   unsigned NumPreds = cast<PHINode>(front()).getNumIncomingValues();
   for (PHINode &Phi : make_early_inc_range(phis())) {
-    Phi.removeIncomingValue(Pred, !KeepOneInputPHIs);
+    Phi.removeIncomingValue(Pred);
     if (KeepOneInputPHIs)
       continue;
     // If we have a single predecessor, removeIncomingValue erased the PHI

diff  --git a/llvm/test/Transforms/LoopRotate/phi-empty.ll b/llvm/test/Transforms/LoopRotate/phi-empty.ll
new file mode 100644
index 000000000000..e246cff91b62
--- /dev/null
+++ b/llvm/test/Transforms/LoopRotate/phi-empty.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -lcssa -loop-rotate < %s | FileCheck %s
+
+define void @PR48296(i1 %cond) {
+; CHECK-LABEL: @PR48296(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[INC:%.*]], label [[LOOP_BACKEDGE:%.*]]
+; CHECK:       loop.backedge:
+; CHECK-NEXT:    br label [[LOOP]]
+; CHECK:       dead:
+; CHECK-NEXT:    unreachable
+; CHECK:       inc:
+; CHECK-NEXT:    br label [[LOOP_BACKEDGE]]
+; CHECK:       return:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %loop
+
+loop:
+  br i1 %cond, label %inc, label %loop
+
+dead:                                        ; No predecessors!
+  br i1 %cond, label %inc, label %return
+
+inc:
+  br label %loop
+
+return:
+  %r = phi i32 [ undef, %dead ]
+  ret void
+}