[PATCH] D28676: Makes incremental dominator calculation in Loop Unroll pass
Michael Kuperstein via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 16 23:06:47 PST 2017
mkuper added a reviewer: efriedma.
mkuper added a comment.
This looks like it has a lot of overlap with https://reviews.llvm.org/D28073.
Please sync with Eli.
One thing https://reviews.llvm.org/D28073 doesn't do is handle peeling (I guess mostly because Eli has better things to do than fix my code :-) ), so that's a part of this patch we want regardless. Added a couple of comments on that.
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:197
Why do you need the cast<Value>()?
Also, I'd a comment here that the reason VMap[IDom->getBlock()] is guaranteed to point to the right block is because the iteration order is RPO, so we've already made a copy of the IDom.
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:221
+ BasicBlock *CurIDom = DT->getNode(Exit)->getIDom()->getBlock();
+ BasicBlock *NewIDom = DT->findNearestCommonDominator(CurIDom, NewLatch);
+ DT->changeImmediateDominator(Exit, NewIDom);
Do we really need to findNearestCommonDominator() here?
IIRC, we are guaranteed to be in loop-simplify form here, which means we have dedicated exit blocks. Can we figure out what the right IDom is without findNearestCommonDominator, a la D28073?
More information about the llvm-commits