[SplitLandingPadPredecessors] Create a PHINode for the original landingpad only if it has some uses

[• chenli]

Summary

This patch adds a check in SplitLandingPadPredecessors to see if the original landingpad instruction has any uses. If not, we don't need to create a PHINode for it in the joint block since it's gonna be a dead code anyway. The motivation for this patch is that we found a bug that SplitLandingPadPredecessors created a PHINode of token type landingpad, which failed the verifier since PHINode can not be token type. However, the created PHINode will never be used in our code pattern. This patch will workaround this bug, and we might add supports in SplitLandingPadPredecessors to handle token type landingpad with uses in the future.

[reames]

lib/Transforms/Utils/BasicBlockUtils.cpp

629–639

I happen to know from talking to you offline that this is about fixing an issue for token types, but that's not obvious in the code. I'd add something to make this clear like an assertion that the type of the PHI being created isn't a token type with a comment talking about the splitting restriction.

[• chenli]

Update patch w.r.t Philip's comments.

[reames]

LGTM

lib/Transforms/Utils/BasicBlockUtils.cpp

void llvm::SplitLandingPadPredecessors(BasicBlock *OrigBB,
   Clone1->setName(Twine("lpad") + Suffix1);
   NewBB1->getInstList().insert(NewBB1->getFirstInsertionPt(), Clone1);
 
   if (NewBB2) {
     Instruction *Clone2 = LPad->clone();
     Clone2->setName(Twine("lpad") + Suffix2);
     NewBB2->getInstList().insert(NewBB2->getFirstInsertionPt(), Clone2);
 
-    // Create a PHI node for the two cloned landingpad instructions.
+    // Create a PHI node for the two cloned landingpad instructions only
+    // if the original landingpad instruction has some uses.
+    if (!LPad->use_empty()) {
+      assert(!LPad->getType()->isTokenTy() &&
+             "Split cannot be applied if LPad is token type. Otherwise an "
+             "invalid PHINode of token type would be created.");
       PHINode *PN = PHINode::Create(LPad->getType(), 2, "lpad.phi", LPad);
       PN->addIncoming(Clone1, NewBB1);
       PN->addIncoming(Clone2, NewBB2);
       LPad->replaceAllUsesWith(PN);
+    }
     LPad->eraseFromParent();
   } else {
     // There is no second clone. Just replace the landing pad with the first
     // clone.
     LPad->replaceAllUsesWith(Clone1);
     LPad->eraseFromParent();
   }
 }

test/Transforms/RewriteStatepointsForGC/two-invokes-one-landingpad.ll

+; RUN: opt %s -rewrite-statepoints-for-gc -rs4gc-use-deopt-bundles -S | FileCheck %s
+
+declare void @some_call(i64 addrspace(1)*)
+
+declare i32 @"dummy_personality_function"()
+
+define i64 addrspace(1)* @test(i64 addrspace(1)* %obj, i64 addrspace(1)* %obj1)
+  gc "statepoint-example"
+  personality i32 ()* @"dummy_personality_function" {
+entry:
+  invoke void @some_call(i64 addrspace(1)* %obj) [ "deopt"() ]
+          to label %second_invoke unwind label %exceptional_return
+
+second_invoke:                                    ; preds = %entry
+  invoke void @some_call(i64 addrspace(1)* %obj) [ "deopt"() ]
+          to label %normal_return unwind label %exceptional_return
+
+normal_return:                                    ; preds = %second_invoke
+  ret i64 addrspace(1)* %obj
+
+; CHECK: exceptional_return1:
+; CHECK-NEXT: %lpad2 = landingpad token
+
+; CHECK: exceptional_return.split-lp:
+; CHECK-NEXT: %lpad.split-lp = landingpad token
+
+; CHECK: exceptional_return:
+; CHECK-NOT: phi token
+
+exceptional_return:                               ; preds = %second_invoke, %entry
+  %lpad = landingpad token cleanup
+  ret i64 addrspace(1)* %obj1
+}