[PATCH] Select Elimination in InstCombine

[David Majnemer]

================
Comment at: lib/Transforms/InstCombine/InstCombine.h:247
@@ -244,1 +246,3 @@
   Instruction *visitInstruction(Instruction &I) { return nullptr; }
+  bool dominatesAllUses(Instruction *DEF, Instruction *USE, BasicBlock *D);
+  bool isChainSelectCmpBranch(SelectInst *SI);
----------------
It's unusual to capitalize all the letters in `DEF` and `USE`, it's more common to see `Def` and `Use`.

I think all arguments and the method could be const.

================
Comment at: lib/Transforms/InstCombine/InstCombine.h:248
@@ -245,1 +247,3 @@
+  bool dominatesAllUses(Instruction *DEF, Instruction *USE, BasicBlock *D);
+  bool isChainSelectCmpBranch(SelectInst *SI);
 
----------------
`SI` could be const as should the method itself.

================
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());
----------------
Could be a little simpler with `for (const Use &U : Def->uses()) {`

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2442-2443
@@ +2441,4 @@
+      continue;
+    DominatesAll = false;
+    break;
+  }
----------------
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.

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

================
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;
+}
----------------
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`?

http://reviews.llvm.org/D5258