[PATCH] D26954: Remove unnecessary IDom check

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/Sink.cpp:174
+    // tree.
+    assert((*I)->getIDom()->getBlock() == Inst->getParent() &&
+           "Broken dominator tree ?");
----------------
I wouldn't even bother with this assert.
Just remove it and commit.


Honestly, this whole code is silly.

If you want to take a whack at it,  the proper (IMHO) way to do this is:

ncdblock  = find nearestcommondominator(blocks that contain uses)

If ncdblock == Inst->getParent(), stop, can't sink.
Otherwise, 
while (!AcceptableTarget(Inst, ncdblock) && ncdblock != Inst->getParent())
,    NCDBlock = getIDom(NCDBlock);

This will sink as low as possible.
You can then remove the allusesdominatedbyblock check from IsAcceptableTarget. Any idom of the ncd must, by definition, still dominate the uses.
Note that when computing NearestCommonDominator(uses), phi uses have to be considered to live at the end of the incoming block they come from to get optimal results.



https://reviews.llvm.org/D26954