[InstCombine] refactor optional bitcasting in matchSelectFromAndOr() into one code path (NFCI)

[spatel]

Summary

This was suggested as a follow-up in D20774. It's less duplicated code, but I'm not sure if it's more readable.

[spatel]

Patch updated:
Clean up the lambda def and variable names, so it's less messy.

[spatel]

On 2nd thought, this patch just exposed the strange split of logic of the original code.
I put all the commuted variants together in:
http://reviews.llvm.org/rL273702

And that makes the code in matchSelectFromAndOr() much easier to read:
http://reviews.llvm.org/rL273706

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

Instruction *InstCombiner::MatchBSwap(BinaryOperator &I) {
   for (auto *Inst : Insts)
     Worklist.Add(Inst);
   return LastInst;
 }
 
 /// We have an expression of the form (A&C)|(B&D).  Check if A is (cond?-1:0)
 /// and either B or D is ~(cond?-1,0) or (cond?0,-1), then we can simplify this
 /// expression to "cond ? C : D or B".
-static Instruction *matchSelectFromAndOr(Value *A, Value *B, Value *C, Value *D,
-                                         InstCombiner::BuilderTy &Builder) {
+static Value *matchSelectFromAndOr(Value *A, Value *B, Value *C, Value *D,
+  // The potential predicate of the select may be bitcasted. In that case, look
+  // through its bitcast and the corresponding bitcast of the 'not' condition.
+  Type *OrigType = A->getType();
+  if (auto *Bitcast = dyn_cast<BitCastInst>(A))
+    A = Bitcast->getOperand(0);
+
   // If A is not a select of -1/0, this cannot match.
   Value *Cond = nullptr;
   if (match(A, m_SExt(m_Value(Cond))) &&
       Cond->getType()->getScalarType()->isIntegerTy(1)) {
 
-    // ((cond ? -1:0) & C) | (B & (cond ? 0:-1)) -> cond ? C : B.
-    if (match(D, m_Not(m_SExt(m_Specific(Cond)))))
-      return SelectInst::Create(Cond, C, B);
-    if (match(D, m_SExt(m_Not(m_Specific(Cond)))))
-      return SelectInst::Create(Cond, C, B);
-
-    // ((cond ? -1:0) & C) | ((cond ? 0:-1) & D) -> cond ? C : D.
-    if (match(B, m_Not(m_SExt(m_Specific(Cond)))))
-      return SelectInst::Create(Cond, C, D);
-    if (match(B, m_SExt(m_Not(m_Specific(Cond)))))
-      return SelectInst::Create(Cond, C, D);
-  }
-
-  // TODO: Refactor the pattern matching above and below so there's less code.
-
-  // The sign-extended boolean condition may be hiding behind a bitcast. In that
-  // case, look for the same patterns as above. However, we need to bitcast the
-  // input operands to the select and bitcast the output of the select to match
-  // the expected types.
-  if (match(A, m_BitCast(m_SExt(m_Value(Cond)))) &&
-      Cond->getType()->getScalarType()->isIntegerTy(1)) {
-
-    Type *SrcType = cast<BitCastInst>(A)->getSrcTy();
+    auto matchSelect = [&](Value *NotA, Value *X) -> Value * {
+      // If the condition was bitcasted, then its 'not' must also be bitcasted.
+      if (A->getType() != OrigType && isa<BitCastInst>(NotA))
+        NotA = cast<BitCastInst>(NotA)->getOperand(0);
+
+      // ((bc Cond) & C) | (X & (bc ~Cond)) --> bc (Cond ? (bc C) : (bc X))
+      if (!match(NotA, m_CombineOr(m_Not(m_SExt(m_Specific(Cond))),
+                                   m_SExt(m_Not(m_Specific(Cond))))))
+        return nullptr;
+
+      // The bitcasts will either all exist or all not exist. The builder will
+      // not create unnecessary casts if the types already match.
+      Value *BitcastC = Builder.CreateBitCast(C, A->getType());
+      Value *BitcastX = Builder.CreateBitCast(X, A->getType());
+      Value *Select = Builder.CreateSelect(Cond, BitcastC, BitcastX);
+      return Builder.CreateBitCast(Select, OrigType);
+    };
 
-    // ((bc Cond) & C) | (B & (bc ~Cond)) --> bc (select Cond, (bc C), (bc B))
-    if (match(D, m_CombineOr(m_BitCast(m_Not(m_SExt(m_Specific(Cond)))),
-                             m_BitCast(m_SExt(m_Not(m_Specific(Cond))))))) {
-      Value *BitcastC = Builder.CreateBitCast(C, SrcType);
-      Value *BitcastB = Builder.CreateBitCast(B, SrcType);
-      Value *Select = Builder.CreateSelect(Cond, BitcastC, BitcastB);
-      return CastInst::Create(Instruction::BitCast, Select, A->getType());
-    }
-
-    // ((bc Cond) & C) | ((bc ~Cond) & D) --> bc (select Cond, (bc C), (bc D))
-    if (match(B, m_CombineOr(m_BitCast(m_Not(m_SExt(m_Specific(Cond)))),
-                             m_BitCast(m_SExt(m_Not(m_Specific(Cond))))))) {
-      Value *BitcastC = Builder.CreateBitCast(C, SrcType);
-      Value *BitcastD = Builder.CreateBitCast(D, SrcType);
-      Value *Select = Builder.CreateSelect(Cond, BitcastC, BitcastD);
-      return CastInst::Create(Instruction::BitCast, Select, A->getType());
-    }
+    if (Value *V = matchSelect(D, B))
+      return V;
+    if (Value *V = matchSelect(B, D))
+      return V;
   }
 
   return nullptr;
 }
 
 /// Fold (icmp)|(icmp) if possible.
 Value *InstCombiner::FoldOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
                                    Instruction *CxtI) {
if (C1 && C2) {  // (A & C1)|(B & C2)
           V2 = Builder->CreateOr(V1, ConstantExpr::getOr(C3, C4), "bitfield");
           return BinaryOperator::CreateAnd(V2,
                                 Builder->getInt(C1->getValue()|C2->getValue()));
         }
       }
     }
 
     // (A & (C0?-1:0)) | (B & ~(C0?-1:0)) ->  C0 ? A : B, and commuted variants.
-    if (Instruction *Match = matchSelectFromAndOr(A, B, C, D, *Builder))
-      return Match;
-    if (Instruction *Match = matchSelectFromAndOr(B, A, D, C, *Builder))
-      return Match;
-    if (Instruction *Match = matchSelectFromAndOr(C, B, A, D, *Builder))
-      return Match;
-    if (Instruction *Match = matchSelectFromAndOr(D, A, B, C, *Builder))
-      return Match;
+    if (Value *V = matchSelectFromAndOr(A, B, C, D, *Builder))
+      return replaceInstUsesWith(I, V);
+    if (Value *V = matchSelectFromAndOr(B, A, D, C, *Builder))
+      return replaceInstUsesWith(I, V);
+    if (Value *V = matchSelectFromAndOr(C, B, A, D, *Builder))
+      return replaceInstUsesWith(I, V);
+    if (Value *V = matchSelectFromAndOr(D, A, B, C, *Builder))
+      return replaceInstUsesWith(I, V);
 
     // ((A&~B)|(~A&B)) -> A^B
     if ((match(C, m_Not(m_Specific(D))) &&
          match(B, m_Not(m_Specific(A)))))
       return BinaryOperator::CreateXor(A, D);
     // ((~B&A)|(~A&B)) -> A^B
     if ((match(A, m_Not(m_Specific(D))) &&
          match(B, m_Not(m_Specific(C)))))