[PATCH] D52508: [InstCombine] Clean up after IndVarSimplify

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1201
+      m_Value(Select));
+  if (match(&I, m_c_BinOp(m_Value(A), m_OneUse(m_Not(m_OneUse(SelectPred))))) &&
+      IsFreeToInvert(B, B->hasOneUse())) {
----------------
`m_Value(A)` will *always* match.
I wonder if would it make sense to swap them around?



================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1205-1210
+    if (cast<SelectInst>(Select)->getTrueValue() == A)
+      return SelectInst::Create(
+          Pred, ConstantInt::getAllOnesValue(A->getType()), AmC);
+    else
+      return SelectInst::Create(Pred, AmC,
+                                ConstantInt::getAllOnesValue(A->getType()));
----------------
How about
```
  Value *TrueVal = ConstantInt::getAllOnesValue(A->getType());
  Value *FalseVal = AmC;
  if (cast<SelectInst>(Select)->getTrueValue() != A)
    std::swap(TrueVal, FalseVal)
  return SelectInst::Create(Pred, TrueVal, FalseVal);
```


https://reviews.llvm.org/D52508