[InstCombine] Remove redundant code from SimplifyUsingDistributiveLaws

[craig.topper]

Summary

The code I've removed here exists in ExpandBinOp in InstSimplify which we call into before SimplifyUsingDistributiveLaws. The code in InstSimplify looks to have been copied from here.

I verified this code doesn't fire on any lit tests. Not that that proves its definitely dead.

[spatel]

LGTM. I was just stepping through a few examples that went down this path, and I couldn't see how these cases could trigger either.

One comment about the naming: I'm not sure if anyone else sees it this way, but when I read "simplifyXXX", I assume that we're not creating new instructions. But that's clearly not true for this function, so maybe it's better to rename it "foldUsingDistributiveLaws" or "combineUsingDistributiveLaws"?

[craig.topper]

Agree the name should be cahnged. I change that as a separate commit.

[craig.topper]

We have several functions in InstCombine called simplifyXXX that create new instructions. For example SimplifyBSWAP and SimplifyAssociativeOrCommutative

[craig.topper]

is this ok to commit?

[spatel]

Yes - just forgot to click the button here in Phab. LGTM.

llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

if (Op0 && RightDistributesOverLeft(Op0->getOpcode(), TopLevelOpcode)) {
     Value *A = Op0->getOperand(0), *B = Op0->getOperand(1), *C = RHS;
     Instruction::BinaryOps InnerOpcode = Op0->getOpcode(); // op'
 
     // Do "A op C" and "B op C" both simplify?
     if (Value *L = SimplifyBinOp(TopLevelOpcode, A, C, DL))
       if (Value *R = SimplifyBinOp(TopLevelOpcode, B, C, DL)) {
         // They do! Return "L op' R".
         ++NumExpand;
-        // If "L op' R" equals "A op' B" then "L op' R" is just the LHS.
-        if ((L == A && R == B) ||
-            (Instruction::isCommutative(InnerOpcode) && L == B && R == A))
-          return Op0;
-        // Otherwise return "L op' R" if it simplifies.
-        if (Value *V = SimplifyBinOp(InnerOpcode, L, R, DL))
-          return V;
-        // Otherwise, create a new instruction.
         C = Builder->CreateBinOp(InnerOpcode, L, R);
         C->takeName(&I);
         return C;
       }
   }
 
   if (Op1 && LeftDistributesOverRight(TopLevelOpcode, Op1->getOpcode())) {
     // The instruction has the form "A op (B op' C)".  See if expanding it out
     // to "(A op B) op' (A op C)" results in simplifications.
     Value *A = LHS, *B = Op1->getOperand(0), *C = Op1->getOperand(1);
     Instruction::BinaryOps InnerOpcode = Op1->getOpcode(); // op'
 
     // Do "A op B" and "A op C" both simplify?
     if (Value *L = SimplifyBinOp(TopLevelOpcode, A, B, DL))
       if (Value *R = SimplifyBinOp(TopLevelOpcode, A, C, DL)) {
         // They do! Return "L op' R".
         ++NumExpand;
-        // If "L op' R" equals "B op' C" then "L op' R" is just the RHS.
-        if ((L == B && R == C) ||
-            (Instruction::isCommutative(InnerOpcode) && L == C && R == B))
-          return Op1;
-        // Otherwise return "L op' R" if it simplifies.
-        if (Value *V = SimplifyBinOp(InnerOpcode, L, R, DL))
-          return V;
-        // Otherwise, create a new instruction.
         A = Builder->CreateBinOp(InnerOpcode, L, R);
         A->takeName(&I);
         return A;
       }
   }
 
   // (op (select (a, c, b)), (select (a, d, b))) -> (select (a, (op c, d), 0))
   // (op (select (a, b, c)), (select (a, b, d))) -> (select (a, 0, (op c, d)))