This is an archive of the discontinued LLVM Phabricator instance.

[LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl.
ClosedPublic

Authored by fhahn on Apr 4 2018, 11:48 AM.

Details

Diff Detail

Event Timeline

efriedma added inline comments.Apr 16 2018, 12:14 PM
lib/Transforms/Scalar/LoopInterchange.cpp
764

This is slightly more generous than getLoopLatchExitBlock; is it okay if the terminator of the exiting block is a switch or invoke? Or do we check that somewhere else?

865

Is it actually possible for OuterExit to be null here?

fhahn updated this revision to Diff 143596.Apr 23 2018, 10:18 AM
fhahn retitled this revision from [LoopInterchange] Use LoopInfo::getExitBlock() instead of manual impl. to [LoopInterchange] Use getExitBlock()/getExitingBlock instead of manual impl..

Thank you very much for having a look Eli and sorry for the delay, I needed to find some time to investigate another failure in the test suite with this change. Together with D45970 and this patch, building the test-suite with LoopInterchange + LTO passes.

I've updated the code to make sure the exiting's block's terminator is a branch instruction.

fhahn added inline comments.Apr 23 2018, 10:24 AM
lib/Transforms/Scalar/LoopInterchange.cpp
764

Thanks for pointing that out, it was not checked anywhere else. I added checks.

865

I had a look and could not spot any other checks ensuring we have a unique exit block.

efriedma added inline comments.Apr 23 2018, 11:55 AM
lib/Transforms/Scalar/LoopInterchange.cpp
562

What is this closing brace doing here?

Also, the indentation looks weird.

865

processLoopList contains an identical check. And I think OuterLoop->getExitingBlock() == OuterLoop->getLoopLatch() && isa<BranchInst>(OuterLoop->getLoopLatch()->getTerminator()) implies the exit is unique.

fhahn updated this revision to Diff 143615.Apr 23 2018, 12:13 PM

Fix brace and remove null checks for getExitBlock()

fhahn added inline comments.Apr 23 2018, 12:16 PM
lib/Transforms/Scalar/LoopInterchange.cpp
562

Argh, sorry about that. Should be fixed now

865

There's one check to ensure the outermost loop in a nest has a unique exit block in processLoopList. Ah yes, the check Latch == single exiting block & isa<BranchInst> should guarantee that we have a unique exit block, thanks!

This revision is now accepted and ready to land.Apr 24 2018, 5:07 PM
This revision was automatically updated to reflect the committed changes.