[PATCH] D24147: Lower consecutive select instructions correctly.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4549
@@ +4548,3 @@
+                                           SmallVector<SelectInst *, 2> &ASI) {
+  for (BasicBlock::iterator It = BasicBlock::iterator(SI);
+       It != SI->getParent()->end(); ++It) {
----------------
Always push SI first and start from the next instruction?

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4558
@@ +4557,3 @@
+  }
+  SmallPtrSet<const Instruction *, 2> INS;
+  for (const auto *SI : ASI) {
----------------
Early return true if ASI size is 1?

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4559
@@ +4558,3 @@
+  SmallPtrSet<const Instruction *, 2> INS;
+  for (const auto *SI : ASI) {
+    const Instruction *I1 = dyn_cast<Instruction>(SI->getTrueValue());
----------------
Probably use a different variable name from the input parameter.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4560
@@ +4559,3 @@
+  for (const auto *SI : ASI) {
+    const Instruction *I1 = dyn_cast<Instruction>(SI->getTrueValue());
+    const Instruction *I2 = dyn_cast<Instruction>(SI->getFalseValue());
----------------
I1 --> TI
I2 --> FI

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4564
@@ +4563,3 @@
+    if (INS.count(I1) || INS.count(I2))
+      return false;
+    INS.insert(SI);
----------------
Add a comment here for explanation.

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4622
@@ +4621,3 @@
+
+  if (!ShouldOptimize)
+    return false;
----------------
Instead of skipping the optimization, It is better to keep the current behavior if those selects can not be grouped -- optimize them one by one. Skipping optimization completely need more discussion and can be done as follow up.


https://reviews.llvm.org/D24147