[llvm] 5921bc4 - [LLParser] Allow zero-input phi nodes

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-08-31T14:24:12+02:00
New Revision: 5921bc42718d3723ab2a4ba412288db51cc198b2

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

LOG: [LLParser] Allow zero-input phi nodes

Zero-input phi nodes are accepted by the verifier and bitcode reader,
but currently rejected by the IR parser. Allow them there as well.

Because phi nodes must have one entry for each predecessor, such
phis can only occur in blocks without predecessors, aka unreachable
code.

Usually, when removing the last predecessor from a block, we also
remove phi nodes in it. However, this is not possible for
invalidation reasons sometimes, which is why we ended up allowing
zero-entry phis at some point in the past. See 9eb2c0113dfe,
D92247 and PR48296 for context.

I've dropped the verifier unit test, because this is now covered
by the regular IR test.

This fixes at least part of https://github.com/llvm/llvm-project/issues/57446.

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

Added: 
    llvm/test/Assembler/zero-input-phi.ll

Modified: 
    llvm/lib/AsmParser/LLParser.cpp
    llvm/unittests/IR/BasicBlockTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index fd502eded0a0..b0d79cfca217 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -6979,21 +6979,22 @@ int LLParser::parsePHI(Instruction *&Inst, PerFunctionState &PFS) {
   Type *Ty = nullptr;  LocTy TypeLoc;
   Value *Op0, *Op1;
 
-  if (parseType(Ty, TypeLoc) ||
-      parseToken(lltok::lsquare, "expected '[' in phi value list") ||
-      parseValue(Ty, Op0, PFS) ||
-      parseToken(lltok::comma, "expected ',' after insertelement value") ||
-      parseValue(Type::getLabelTy(Context), Op1, PFS) ||
-      parseToken(lltok::rsquare, "expected ']' in phi value list"))
+  if (parseType(Ty, TypeLoc))
     return true;
 
+  if (!Ty->isFirstClassType())
+    return error(TypeLoc, "phi node must have first class type");
+
+  bool First = true;
   bool AteExtraComma = false;
   SmallVector<std::pair<Value*, BasicBlock*>, 16> PHIVals;
 
   while (true) {
-    PHIVals.push_back(std::make_pair(Op0, cast<BasicBlock>(Op1)));
-
-    if (!EatIfPresent(lltok::comma))
+    if (First) {
+      if (Lex.getKind() != lltok::lsquare)
+        break;
+      First = false;
+    } else if (!EatIfPresent(lltok::comma))
       break;
 
     if (Lex.getKind() == lltok::MetadataVar) {
@@ -7007,10 +7008,9 @@ int LLParser::parsePHI(Instruction *&Inst, PerFunctionState &PFS) {
         parseValue(Type::getLabelTy(Context), Op1, PFS) ||
         parseToken(lltok::rsquare, "expected ']' in phi value list"))
       return true;
-  }
 
-  if (!Ty->isFirstClassType())
-    return error(TypeLoc, "phi node must have first class type");
+    PHIVals.push_back(std::make_pair(Op0, cast<BasicBlock>(Op1)));
+  }
 
   PHINode *PN = PHINode::Create(Ty, PHIVals.size());
   for (unsigned i = 0, e = PHIVals.size(); i != e; ++i)

diff  --git a/llvm/test/Assembler/zero-input-phi.ll b/llvm/test/Assembler/zero-input-phi.ll
new file mode 100644
index 000000000000..55489b943b50
--- /dev/null
+++ b/llvm/test/Assembler/zero-input-phi.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S < %s | FileCheck %s
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
+define void @dead_phi() {
+; CHECK-LABEL: @dead_phi(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret void
+; CHECK:       return:
+; CHECK-NEXT:    [[R:%.*]] = phi i32
+; CHECK-NEXT:    ret void
+;
+entry:
+  ret void
+
+return:
+  %r = phi i32
+  ret void
+}

diff  --git a/llvm/unittests/IR/BasicBlockTest.cpp b/llvm/unittests/IR/BasicBlockTest.cpp
index 408275732058..b364b981b403 100644
--- a/llvm/unittests/IR/BasicBlockTest.cpp
+++ b/llvm/unittests/IR/BasicBlockTest.cpp
@@ -169,23 +169,6 @@ TEST(BasicBlockTest, ComesBefore) {
   EXPECT_FALSE(Ret->comesBefore(Ret));
 }
 
-TEST(BasicBlockTest, EmptyPhi) {
-  LLVMContext Ctx;
-
-  Module *M = new Module("MyModule", Ctx);
-  FunctionType *FT = FunctionType::get(Type::getVoidTy(Ctx), {}, false);
-  Function *F = Function::Create(FT, Function::ExternalLinkage, "", M);
-
-  BasicBlock *BB1 = BasicBlock::Create(Ctx, "", F);
-  ReturnInst::Create(Ctx, BB1);
-
-  Type *Ty = Type::getInt32PtrTy(Ctx);
-  BasicBlock *BB2 = BasicBlock::Create(Ctx, "", F);
-  PHINode::Create(Ty, 0, "", BB2);
-  ReturnInst::Create(Ctx, BB2);
-  EXPECT_FALSE(verifyModule(*M, &errs()));
-}
-
 class InstrOrderInvalidationTest : public ::testing::Test {
 protected:
   void SetUp() override {