[PATCH] Select Elimination in InstCombine

Gerolf Hoflehner ghoflehner at apple.com
Wed Sep 10 20:04:40 PDT 2014


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2437-2438
@@ +2436,4 @@
+  bool DominatesAll = true;
+  for (auto I = DEF->use_begin(), E = DEF->use_end(); I != E; ++I) {
+    Use &U = *I;
+    auto *User = cast<Instruction>(U.getUser());
----------------
majnemer wrote:
> Could be a little simpler with `for (const Use &U : Def->uses()) {`
Done

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2442-2443
@@ +2441,4 @@
+      continue;
+    DominatesAll = false;
+    break;
+  }
----------------
majnemer wrote:
> You could make this a little easier to follow if you made this  `return false;`
> 
> In fact, you could probably remove the `continue;` as well.
Done

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2445
@@ +2444,3 @@
+  }
+  return DominatesAll;
+}
----------------
majnemer wrote:
> This could just be `return true;`
Done

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2459-2461
@@ +2458,5 @@
+  auto *IC = dyn_cast<ICmpInst>(BI->getCondition());
+  if (!IC || IC->getOperand(0) != SI)
+    return false;
+  return true;
+}
----------------
majnemer wrote:
> You could make this `return IC && IC->getOperand(0) == SI;`
> 
> Are we interested in doing anything if the other operand to the `ICmpInst` is the `SelectInst`?
No, rhs is expected to be a constant.

http://reviews.llvm.org/D5258






More information about the llvm-commits mailing list