[PATCH] Select Elimination in InstCombine

Eric Christopher echristo at gmail.com
Tue Sep 9 00:52:34 PDT 2014


Hi Gerolf,

One more comment in line.

As far as the dominance tree, you don't ever check to find out if you have dominance trees, you just require it. The leading question was: if it's not optional, then you can probably use it in more places in the pass. If it is optional then you're not actually checking for it before using it.

Thanks!

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2977
@@ +2976,3 @@
+              // replace select with operand on then path for NE compares.
+              int Path = I.getPredicate() == ICmpInst::ICMP_EQ ? 1 : 0;
+              if (InstCombiner::dominatesAllUses(
----------------
majnemer wrote:
> echristo wrote:
> > Isn't this just a boolean then?
> I'd make this an unsigned, getSuccessor is asking which successor to take.  It just so happens that we are only interested in cases where there are two successors.
Yes. Makes sense that way. Might be better off just grabbing the correct successor rather than mucking with the integer.

http://reviews.llvm.org/D5258






More information about the llvm-commits mailing list