[PATCH] Select elimination pass

[Juergen Ributzka]

Hi Gerolf,

I added a few comments and questions inline.

-Juergen

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:80
@@ +79,3 @@
+    AU.addRequired<DominatorTreeWrapperPass>();
+    AU.addPreserved<DominatorTreeWrapperPass>();
+  }
----------------
Your pass doesn't preserve the CFG?

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:82
@@ +81,3 @@
+  }
+  // dominatesAllUses - true when \param I dominates all of its uses.
+  //
----------------
We don't add the function name in the comments anymore.

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:105
@@ +104,3 @@
+///
+/// \brief createSelectEliminationPass - The public interface to this file.
+///
----------------
ditto

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:118
@@ +117,3 @@
+///
+/// \brief isChainSelectCmpEQBranch - true when there is an icmp-select-icmp
+/// sequence in a basic block
----------------
ditto

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:145
@@ +144,3 @@
+///
+/// \brief canReplaceSelectWithOr - true when the false value of the select
+/// and the second operand of icmp.eq are the null pointer, and all
----------------
ditto

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:170
@@ +169,3 @@
+///
+/// \brief replaceSelectWithOr - converts icmp.eq - select - icmp.eq
+/// to icmp.ne - icmp.eq - or
----------------
ditto

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:175
@@ +174,3 @@
+  DEBUG(dbgs() << "ICMP - SELECT - ICMP pattern found!\n";);
+  auto IcmpD = dyn_cast<ICmpInst>(SI->getCondition());
+  IcmpD->setPredicate(ICmpInst::ICMP_NE);
----------------
I would add an assert with isa<ICmpInst> and then use just cast<ICmpInst>, since you are not checking the result of the dyn_cast anyways.

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:212
@@ +211,3 @@
+      Instruction *I = &*II;
+      II++;
+      if (isa<SelectInst>(I)) {
----------------
Why is the iterator incremented in the loop?

================
Comment at: lib/Transforms/Scalar/SelectElimination.cpp:214
@@ +213,3 @@
+      if (isa<SelectInst>(I)) {
+        auto SI = cast<SelectInst>(I);
+        if (isChainSelectCmpEQBranch(SI)) {
----------------
You could combine this with dyn_cast to one line or you could invert the condition and add a continue to reduce the indentation of the code afterwards.

http://reviews.llvm.org/D5156