[PATCH] Select Elimination in InstCombine

Gerolf Hoflehner ghoflehner at apple.com
Tue Nov 11 20:37:37 PST 2014


================
Comment at: include/llvm/IR/Value.h:264
@@ +263,3 @@
+  /// block
+  ///
+  void replaceUsesOutsideBlock(Value *V, BasicBlock *ExceptionBB);
----------------
hfinkel wrote:
> Add period after block and remove empty comment line.
> 
> The other differences from RAUW (as noted above the implementation) should also be noted here.
> 
> There is no need to name the BB argument ExceptionBB, BB is fine.
ok.

================
Comment at: lib/IR/Value.cpp:360
@@ +359,3 @@
+// This routine leaves uses within ExceptionBB.
+void Value::replaceUsesOutsideBlock(Value *New, BasicBlock *ExceptionBB) {
+  assert(New && "Value::replaceUsesOutsideBlock(<null>) is invalid!");
----------------
hfinkel wrote:
> We don't need to use the name ExceptionBB, or refer to this as the "exception block" in the assert below.
ok.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2566
@@ +2565,3 @@
+    if (InstCombiner::dominatesAllUses(SI, Icmp, Succ) &&
+        Succ->getUniquePredecessor()) {
+      NumSel++;
----------------
hfinkel wrote:
> I assume that Succ->getUniquePredecessor() is cheaper to evaluate than InstCombiner::dominatesAllUses(SI, Icmp, Succ), so I'd check that first. Also, you can drop the InstCombiner::
Good point. At least your assumption should be right :-)

http://reviews.llvm.org/D5258






More information about the llvm-commits mailing list