[PATCH] Fixing inst-combine not to drops nsw when combining adds into mul (PR19263)

[Jingyue Wu]

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:466
@@ +465,3 @@
+      bool InnerCommutative = Instruction::isCommutative(LHSOpcode);
+      // Does "X op' (Y op Z)" always equal "(X op' Y) op (X op' Z)"?
+      if (LeftDistributesOverRight(LHSOpcode, TopLevelOpcode))
----------------
Dinesh Dwivedi wrote:
> Jingyue Wu wrote:
> > I feel the logic of this IF and next IF (if (RightDistributesOverLeft) a bit difficult to follow. There is also some copy and paste. What about extracting the check on InnerCommutative out? Something like:
> > ```
> > try(a, b, c, d);
> > if (op' is commutative) {
> >   try(b, a, c, d);
> >   try(a, b, d, c);
> >   try(b, a, c, d);
> > }
> > ```
> > By doing this, inside each "try", we don't need to think about whether op' is commutative. 
> > 
> > What do you think?
> Updated patch to handle this. We can not just depend on isCommutative as
> there are oparation which are not commutative but still can be distributed e.g.
> "(X + Y) / Z = X/Z + Y/Z"
> 
> We do not handle this now but we might want to do it in future.
I think the first "try" in my pseudo-code handle this case. Right? 

While "try" doesn't swap a and b or c and d, it considers both LeftDistributesOverRight and RightDistributesOverLeft. Therefore, it is able to convert "a / b + c / d" to "(a + c) / d".

http://reviews.llvm.org/D3799