[PATCH] Select Elimination in InstCombine

Gerolf Hoflehner ghoflehner at apple.com
Tue Sep 30 17:09:59 PDT 2014


================
Comment at: lib/Transforms/InstCombine/InstCombine.h:102
@@ -101,2 +101,3 @@
   DominatorTree *DT; // not required
+  TargetLibraryInfo *TLI;
   bool MadeIRChange;
----------------
majnemer wrote:
> This change looks superfluous.
Good catch. Somehow I flipped TLI and DT. Done.
Also, the not // not required comment on DT is wrong now, so I removed it.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2452
@@ +2451,3 @@
+    auto *Usr = cast<Instruction>(U);
+    if (Usr != UI && !DT->dominates(DB, Usr->getParent()))
+      return false;
----------------
hfinkel wrote:
> Gerolf wrote:
> > Gerolf wrote:
> > > hfinkel wrote:
> > > > If you mean for this to rule-out any other uses in the same block as the definition (I don't see anything else that might enforce that there is only one use in the parent block), should this be a call to properlyDominates?
> > > Very much agree. Done.
> > Ups, I have to take it back. DB can be a successor block of the DI parent and it may contain a use. So dominates() must be used. 
> Alright, please make sure the function description comment is accurate and that this is noted.
Ok. I sharpened the comment and explicitly mention that there could a use of \p DI in \p DB.

http://reviews.llvm.org/D5258






More information about the llvm-commits mailing list