This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by chenli on Dec 30 2015, 10:04 PM.

Details

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.

Diff Detail

Event Timeline

chenli retitled this revision from to [SplitLandingPadPredecessors] Create a PHINode for the original landingpad only if it has some uses.
chenli updated this object.
chenli added a reviewer: reames.
chenli added a subscriber: llvm-commits.
reames requested changes to this revision.Jan 4 2016, 1:58 PM
reames edited edge metadata.
reames added inline comments.
lib/Transforms/Utils/BasicBlockUtils.cpp
629

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.

This revision now requires changes to proceed.Jan 4 2016, 1:58 PM
chenli updated this revision to Diff 44092.Jan 5 2016, 10:11 PM
chenli updated this object.
chenli edited edge metadata.
chenli removed a subscriber: sanjoy.

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

reames accepted this revision.Jan 6 2016, 11:38 AM
reames edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 6 2016, 11:38 AM
chenli closed this revision.Jan 6 2016, 12:35 PM