Page MenuHomePhabricator

[IROutliner] Ignore Regions where part of an outlined phi nodes incoming block is included, but the final branch instruction is not
ClosedPublic

Authored by AndrewLitteken on Mar 9 2022, 10:56 AM.

Details

Summary

Reproduced: https://godbolt.org/z/roznP99P3

When outlining a phi node, if the the incoming branch is a block contained in the region and the branch from that block is not outlined, we create broken code. The fix is to recognize when that branch from the included incoming block is not contained, and ignore the region.

Diff Detail

Event Timeline

AndrewLitteken created this revision.Mar 9 2022, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2022, 10:56 AM
AndrewLitteken requested review of this revision.Mar 9 2022, 10:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2022, 10:56 AM
AndrewLitteken set the repository for this revision to rG LLVM Github Monorepo.
paquette added inline comments.Mar 9 2022, 11:33 AM
llvm/lib/Transforms/IPO/IROutliner.cpp
280

since we are unable to handle the severing of the phi node right now.

Should this be a TODO?

309

Nit: Pull PN->getIncomingBlock(i) into a variable?

310

Isn't this always true since EndBB = BackInst->getParent()? Could this check be removed?

311

I think this could be hoisted out of the loops and into a variable?

bool EndBBTermAndBackInstDifferent = EndBB->getTerminator() != BackInst;
while (PhiNode ...)
317–318

Nit: This can all be one if now:

if (isa<PHINode>(BackInst) && BackInst != &*std::prev(EndBB->getFirstInsertionPt()))
   return;
paquette added inline comments.Mar 14 2022, 11:03 AM
llvm/lib/Transforms/IPO/IROutliner.cpp
300

Pull this above the if (!BBSet.contains(PN->getIncomingBlock(i)))?

Also, which GitHub issue is this associated with?

paquette accepted this revision.Mar 16 2022, 10:36 AM

I think after the one nit is fixed, this is good to go.

This revision is now accepted and ready to land.Mar 16 2022, 10:36 AM