[llvm-commits] [llvm] r120024 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineAddSub.cpp lib/Transforms/InstCombine/InstCombineAndOrXor.cpp lib/Transforms/InstCombine/InstructionCombining.cpp test/

[Duncan Sands]

Hi Frits,

> I believe you typo'd "(X LOp Y) ROp (X LOp Z)" here.

yup - fixed.

>> +  // Does "X op' (Y op Z)" always equal "(X op' Y) op (X op' Z)"?
>> +  bool LeftDistributes = LeftDistributesOverRight(InnerOpcode, OuterOpcode);
>> +  // Does "(X op Y) op' Z" always equal "(X op' Z) op (Y op' Z)"?
>> +  bool RightDistributes = RightDistributesOverLeft(OuterOpcode, InnerOpcode);
>
> Why are these two here instead of in the if() where they're tested?
> They don't seem to be reused...

Just trying to prime peoples brains with all the essential information before
getting into the guts of it.  But I agree that it reads better the way you
suggest, so I've changed it.

>> +      // If "B op D" doesn't simplify then only proceed if both of the existing
>> +      // operations "A op' B" and "C op' D" will be zapped since no longer used.
>
> You typo'd "since /they're/ no longer used" here.

It might be grammatically a little naughty, but I think it is OK.  The advantage
is that this way the comment fits onto two lines nicely formatted :)

>> +      // If "A op C" doesn't simplify then only proceed if both of the existing
>> +      // operations "A op' B" and "C op' D" will be zapped since no longer used.

Likewise.

Ciao,

Duncan.