[PATCH] D37019: Add select simplifications

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:731-733
+  if (match(LHS, m_Select(m_Value(A1), m_Value(B), m_Value(C))) &&
+      match(RHS, m_Select(m_Value(A2), m_Value(D), m_Value(E)))) {
+    if (A1 == A2) {
----------------
You can simplify this a little using `m_Specific`:
  if (match(LHS, m_Select(m_Value(A), m_Value(B), m_Value(C))) &&
      match(RHS, m_Select(m_Specific(A), m_Value(D), m_Value(E)))) {

This also lets us remove a level of indentation.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:739
+      Value *V1 = SimplifyBinOp(Opcode, C, E, SQ.getWithInstruction(&I));
+      Value *SI = (V1) ? Builder.CreateSelect(A1, Builder.CreateBinOp(Opcode, B, D), V1) : nullptr;
+      if (Value *V2 = SimplifyBinOp(Opcode, B, D, SQ.getWithInstruction(&I)))
----------------
majnemer wrote:
> It is uncommon in LLVM to parenthesize the condition of a ternary expression.
Are there situations where we'd unnecessarily `CreateSelect`?

My reading of the code is that we will throw away this `SelectInst` if both `V1` and `V2` end up being true.

Maybe something like the following would work:
  Value *SI = nullptr;
  Value *V1 = SimplifyBinOp(Opcode, C, E, SQ.getWithInstruction(&I));
  Value *V2 = SimplifyBinOp(Opcode, B, D, SQ.getWithInstruction(&I));
  if (V1 && V2) {
    SI = Builder.CreateSelect(A1, V2, V1);
  } else if (V2) {
    SI = Builder.CreateSelect(A1, V2, Builder.CreateBinOp(Opcode, C, E));
  } else if (V1) {
    SI = Builder.CreateSelect(A1, Builder.CreateBinOp(Opcode, B, D), V1);
  }

  if (SI) {
    SI->takeName(&I);
    return SI;
  }


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:739-741
+      Value *SI = (V1) ? Builder.CreateSelect(A1, Builder.CreateBinOp(Opcode, B, D), V1) : nullptr;
+      if (Value *V2 = SimplifyBinOp(Opcode, B, D, SQ.getWithInstruction(&I)))
+        SI = (V1) ? Builder.CreateSelect(A1, V2, V1) :
----------------
It is uncommon in LLVM to parenthesize the condition of a ternary expression.


https://reviews.llvm.org/D37019