[PATCH] D133000: [LLParser] Allow zero-input phi nodes

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: spatel, regehr.
Herald added a subscriber: hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.

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.


https://reviews.llvm.org/D133000

Files:
  llvm/lib/AsmParser/LLParser.cpp
  llvm/test/Assembler/zero-input-phi.ll
  llvm/unittests/IR/BasicBlockTest.cpp


Index: llvm/unittests/IR/BasicBlockTest.cpp
===================================================================
--- llvm/unittests/IR/BasicBlockTest.cpp
+++ llvm/unittests/IR/BasicBlockTest.cpp
@@ -169,23 +169,6 @@
   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 {
Index: llvm/test/Assembler/zero-input-phi.ll
===================================================================
--- /dev/null
+++ 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
+}
Index: llvm/lib/AsmParser/LLParser.cpp
===================================================================
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -6979,21 +6979,19 @@
   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;
 
+  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,6 +7005,8 @@
         parseValue(Type::getLabelTy(Context), Op1, PFS) ||
         parseToken(lltok::rsquare, "expected ']' in phi value list"))
       return true;
+
+    PHIVals.push_back(std::make_pair(Op0, cast<BasicBlock>(Op1)));
   }
 
   if (!Ty->isFirstClassType())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133000.456879.patch
Type: text/x-patch
Size: 2902 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220831/012f735d/attachment.bin>