[PATCH] Added instruction combine to transform few more negative values addition to subtraction

[Dinesh Dwivedi]

Hi Philip,

I have added one comment at http://reviews.llvm.org/D3733 to reply your queries. Will you please
check it and let me know if it is clear or not ?

Regards
Dinesh Dwivedi

------- Original Message -------
Sender : Philip Reames<listmail at philipreames.com> 
Date   : May 27, 2014 22:14 (GMT+05:30)
Title  : Re: [PATCH] Added instruction combine to transform few more negative
 values addition to subtraction

Glancing at your patch, a few quick comments.

// Checks if any operand is negative and we can convert add to sub.
// This function checks for following patterns
// ADD(XOR(OR(Z, NOT(C)), C)), 1) == NEG(AND(Z, C))
// ADD(XOR(AND(Z, ~C), ~C), 1) == NEG(OR(Z, C)) if C is even
// XOR(AND(Z, ~C), (~C + 1)) == NEG(OR(Z, C)) if C is odd
Value *checkForNegativeOperand(BinaryOperator &I, 
InstCombiner::BuilderTy *Builder) { ...}

Both your comment and function name here seem misleading.  As far as I 
can tell from your transforms, none are actual predicated on an argument 
being negative.  I also see no use of subtraction.

Then for this one, I'm probably missing something obvious, but where are 
you initializing C1 and C2?  In general, the code structure here is a 
bit hard to read.


Philip

http://reviews.llvm.org/D3733