[PATCH] D15232: [InstCombine] Aggressively fold compares that can be discovered to be constant

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3049
@@ +3048,3 @@
+
+    if (SelectInst *SI = dyn_cast<SelectInst>(P)) {
+      Worklist.push_back(SI->getTrueValue());
----------------
`auto *`

================
Comment at: lib/Analysis/ValueTracking.cpp:3055
@@ +3054,3 @@
+
+    if (PHINode *PN = dyn_cast<PHINode>(P)) {
+      for (Value *IncValue : PN->incoming_values())
----------------
`auto *`

================
Comment at: lib/Analysis/ValueTracking.cpp:3062
@@ +3061,3 @@
+    if (MaxRecurse > 1)
+      if (BinaryOperator *BO = dyn_cast<BinaryOperator>(P))
+        // If this operator has a RHS of a constant int, we can possibly
----------------
`auto *`

================
Comment at: lib/Analysis/ValueTracking.cpp:3065
@@ +3064,3 @@
+        // constant fold it if the LHS turns out also to be a constant.
+        if (ConstantInt *RHS = dyn_cast<ConstantInt>(BO->getOperand(1))) {
+          SmallVector<Value *, 4> Vs;
----------------
`auto *`

================
Comment at: lib/Analysis/ValueTracking.cpp:3068-3069
@@ +3067,4 @@
+          GetUnderlyingValues(BO->getOperand(0), Vs, DL, MaxRecurse - 1);
+          if (std::all_of(Vs.begin(), Vs.end(), [](const Value *V) {
+                return isa<ConstantInt>(V);
+              })) {
----------------
Does `isa<ConstantInt>` fulfill `UnaryPredicate`?  If so, you don't need the lambda.

================
Comment at: lib/Analysis/ValueTracking.cpp:3077-3082
@@ +3076,8 @@
+                                           {cast<ConstantInt>(V), RHS}, DL);
+              if (!C)
+                Fail = true;
+              else
+                NewVs.push_back(C);
+            }
+            if (!Fail) {
+              Worklist.insert(Worklist.end(), NewVs.begin(), NewVs.end());
----------------
It doesn't look like `NewVs` will be used if `Fail` is true. Why not break out of the loop?  

================
Comment at: lib/Analysis/ValueTracking.cpp:3090
@@ +3089,3 @@
+    // See if InstructionSimplify knows any relevant tricks.
+    if (Instruction *I = dyn_cast<Instruction>(P))
+      // TODO: Acquire a DominatorTree and AssumptionCache and use them.
----------------
`auto *`

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2764
@@ +2763,3 @@
+  // fold the compare.
+  if (ConstantInt *CRHS = dyn_cast<ConstantInt>(Op1)) {
+    SmallVector<Value *, 4> Values;
----------------
`auto *`


Repository:
  rL LLVM

http://reviews.llvm.org/D15232