[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



On 05/26/2014 04:42 AM, Dinesh Dwivedi wrote:
> gentle ping.
>
> Will it help in reviewing if I break this patch in parts.
>
> http://reviews.llvm.org/D3733
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits