Page MenuHomePhabricator

[IfConversion] Correctly handle cases where analyzeBranch fails.

Authored by efriedma on Sep 6 2019, 4:14 PM.



If analyzeBranch fails, on some targets, the out parameters point to some blocks in the function. But we can't use that information, so make sure to clear it out. (In some places in IfConversion, we assume that any block with a TrueBB is analyzable.)

The change to the testcase makes it trigger a bug on builds without this fix: IfConvertDiamond tries to perform a followup "merge" operation, which isn't legal, and we somehow end up with a branch to a deleted MBB. I'm not sure how this doesn't crash the compiler.


Diff Detail


Event Timeline

efriedma created this revision.Sep 6 2019, 4:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2019, 4:14 PM

FWIW, I verified that this patch fixes pr/42012 and the test case pases. Thanks @eli.friedman !

hans added a comment.Sep 9 2019, 7:42 AM

Reviewers, please try to prioritize this one, since it's currently the last patch blocking llvm 9-rc4.

dmgreen accepted this revision.Sep 9 2019, 8:02 AM

Sorry, yes, I think this makes sense over the alternative of analyzeBranch not setting TrueBB/FalseBB. LGTM

This revision is now accepted and ready to land.Sep 9 2019, 8:02 AM
This revision was automatically updated to reflect the committed changes.