[llvm-commits] patch: folding (zext (and x, cst))

[Nick Lewycky]

Eli Friedman wrote:
> On Tue, Jun 7, 2011 at 2:46 AM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>> I have an unfinished patch. I was looking to optimize:
>>
>>   define i32 @test1(i8 %x) nounwind readnone {
>>     %A = and i8 %x, -32
>>     %B = zext i8 %A to i32
>>     ret i32 %B
>>   }
>>
>> which currently does a mov into %al, then the "and", then extends, into
>> doing a single extending mov, then an "and" in 32-bits. The rule I decided
>> upon is "(zext (and x, cst)) ->  (and (anyext x), (zext cst))" in the DAG
>> combiner.
>
> Just a comment, without reading the patch: it would be much more
> conservative to fold (zext (and (load x), cst)) ->  (and (zextload x),
> (zext cst)).  The transform you're proposing is much less obviously
> beneficial.

Sure, but the transform I proposed should turn into this one. 
Regardless, since I wasn't able to resolve the collateral issues brought 
up by my first patch, I've implemented the transform you described and 
its sext version.

This is totally cargo-cult written, and I'm not sure what I'm doing 
wrong. Do I need the safety check I have for !LN0->isVolatile()? Should 
I have a check for !LegalOperations? How about some calls to CombineTo?

Patch attached, please review! Carefully! ;)

Once this lands, I can make some instcombine changes that will make this 
fire more often, I just don't want to start pessimizing code in the 
middle-end.

Nick

>
> -Eli
>
>> That wasn't so hard to implement, but I haven't been able to chase down all
>> of the fallout. The primary problem is that any-extends are showing up in
>> places that didn't expect them. I added another DAG combine to fold away an
>> anyext in one transform with sbbl, but then got rather stuck.
>>
>> The patterns added in this commit:
>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110502/120547.html
>> isn't matching any more. The relevant portion of before:
>>
>>         0x90580f0: i32 = X86ISD::CMP 0x9057a90, 0x9057a08
>>       0x9057f58: i32 = X86ISD::SETCC_CARRY 0x9057fe0, 0x90580f0
>>     0x9058068: i32 = sub 0x9057a90, 0x9057f58
>>
>> and after:
>>
>>           0x96ff548: i32 = X86ISD::CMP 0x96feee8, 0x96fee60
>>         0x96ff4c0: i8 = X86ISD::SETCC_CARRY 0x96ff438, 0x96ff548
>>       0x96fef70: i32 = sign_extend 0x96ff4c0
>>     0x96ff080: i32 = sub 0x96feee8, 0x96fef70
>>
>> I'm not sure why this isn't being folded away, because X86InstrCompiler.td
>> has a pattern that matches anyext(X86setcc_c) and turns it into a single
>> X86setcc_c of the extended type. Why isn't that firing here?
>>
>> I've attached my patch and the result of running 'llc -debug
>> test/CodeGen/X86/add-of-carry.ll' with my patch in place.
>>
>> Nick
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ext-and-load.patch
Type: text/x-patch
Size: 3916 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110615/a958f2ae/attachment.bin>