[llvm-commits] patch: folding (zext (and x, cst))
Eli Friedman
eli.friedman at gmail.com
Wed Jun 15 08:35:56 PDT 2011
On Wed, Jun 15, 2011 at 3:24 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
> 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()?
No, don't think so; replacing a volatile load with a volatile load
should be okay.
> Should I have a
> check for !LegalOperations?
You need a check like "(!LegalOperations ||
TLI.isOperationLegal(ISD::AND, VT))" to make sure you aren't creating
an illegal AND.
> How about some calls to CombineTo?
You need a CombineTo call to handle the chain on the load.
> Patch attached, please review! Carefully! ;)
Your use of ExtendUsesToFormExtLoad (and the !N0.hasOneUse() case in
general) looks suspicious.
I don't see any obvious issues besides the ones mentioned above.
-Eli
More information about the llvm-commits
mailing list