From bfd2c216ea8ef09f8fb1f755ca2b89f86f74acbb Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Mon, 30 Nov 2020 08:59:40 -0500 Subject: [PATCH] [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 --- llvm/include/llvm/IR/BasicBlock.h | 6 ++-- llvm/lib/IR/BasicBlock.cpp | 2 +- llvm/test/Transforms/LoopRotate/phi-empty.ll | 34 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 llvm/test/Transforms/LoopRotate/phi-empty.ll diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h index 26cfdd9e51d69..149b0a26c1f3c 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 3268641ddf194..aee769aa0fea7 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(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 0000000000000..e246cff91b620 --- /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 +}