[PATCH] Select Elimination in InstCombine
Eric Christopher
echristo at gmail.com
Mon Sep 8 22:14:21 PDT 2014
A few nits in line, one question in line, and one more thought: Can't this replace a few of our other queries for instructions with dominator tree lookups? Is that a follow on patch or?
Thanks!
-eric
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2429
@@ +2428,3 @@
+//
+// true when \param SI dominates all of its uses or \param ICmp.
+//
----------------
s/or/of
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2433
@@ +2432,3 @@
+ BasicBlock *D) {
+ bool dominates_all_uses = true;
+ for (auto I = SI->use_begin(), E = SI->use_end(); I != E; ++I) {
----------------
Local variable naming style.
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2945
@@ +2944,3 @@
+
+ bool transform = false;
+ SelectInst *SI = cast<SelectInst>(LHSI);
----------------
Local variable naming style.
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2975
@@ +2974,3 @@
+ if (ReplaceWithOpd && IsEQorNECompare) {
+ // replace select with operand on else path for EQ compares.
+ // replace select with operand on then path for NE compares.
----------------
Nit: Sentences begin with capital letters.
================
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(
----------------
Isn't this just a boolean then?
================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:95
@@ -94,1 +94,3 @@
AU.addRequired<TargetLibraryInfo>();
+ AU.addRequired<DominatorTreeWrapperPass>();
+ AU.addPreserved<DominatorTreeWrapperPass>();
----------------
Probably don't need to have it as requires, but optional? i.e. if it doesn't exist then just don't do the transform in the way you've got?
http://reviews.llvm.org/D5258
More information about the llvm-commits
mailing list