<div class="gmail_quote">On 15 June 2011 08:35, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<div class="im">On Wed, Jun 15, 2011 at 3:24 AM, Nick Lewycky <<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>> wrote:<br>
> Eli Friedman wrote:<br>
>><br>
>> On Tue, Jun 7, 2011 at 2:46 AM, Nick Lewycky<<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>>  wrote:<br>
>>><br>
>>> I have an unfinished patch. I was looking to optimize:<br>
>>><br>
>>>  define i32 @test1(i8 %x) nounwind readnone {<br>
>>>    %A = and i8 %x, -32<br>
>>>    %B = zext i8 %A to i32<br>
>>>    ret i32 %B<br>
>>>  }<br>
>>><br>
>>> which currently does a mov into %al, then the "and", then extends, into<br>
>>> doing a single extending mov, then an "and" in 32-bits. The rule I<br>
>>> decided<br>
>>> upon is "(zext (and x, cst)) ->  (and (anyext x), (zext cst))" in the DAG<br>
>>> combiner.<br>
>><br>
>> Just a comment, without reading the patch: it would be much more<br>
>> conservative to fold (zext (and (load x), cst)) ->  (and (zextload x),<br>
>> (zext cst)).  The transform you're proposing is much less obviously<br>
>> beneficial.<br>
><br>
> Sure, but the transform I proposed should turn into this one. Regardless,<br>
> since I wasn't able to resolve the collateral issues brought up by my first<br>
> patch, I've implemented the transform you described and its sext version.<br>
><br>
> This is totally cargo-cult written, and I'm not sure what I'm doing wrong.<br>
> Do I need the safety check I have for !LN0->isVolatile()?<br>
<br>
</div>No, don't think so; replacing a volatile load with a volatile load<br>
should be okay.<br></blockquote><div><br></div><div>Ok, thanks.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">> Should I have a<br>
> check for !LegalOperations?<br>
<br>
</div>You need a check like "(!LegalOperations ||<br>
TLI.isOperationLegal(ISD::AND, VT))" to make sure you aren't creating<br>
an illegal AND.<br></blockquote><div><br></div><div>Done.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">> How about some calls to CombineTo?<br>
<br>
</div>You need a CombineTo call to handle the chain on the load.<br></blockquote><div><br></div><div>Okay, but CombineTo is undocumented. It appears to be CombineTo(Old, New) to replace Old with New, so I tried calling it with CombineTo(LN0, ExtLoad) but that triggered the assert on the first line of CombineTo. Could you explain a little more about what I need to do?</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">> Patch attached, please review! Carefully! ;)<br>
<br>
</div>Your use of ExtendUsesToFormExtLoad (and the !N0.hasOneUse() case in<br>
general) looks suspicious.<br></blockquote><div><br></div><div>Cargo-culted from nearby. Look at the way "fold (zext (load x)) -> (zext (truncate (zextload x)))" is handled. I think what's going on is that the (load x) may have other uses, so the ExtendUsesToFormExtLoad function checks whether inserting the necessary truncates would be free or the users could be extended, etc.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">I don't see any obvious issues besides the ones mentioned above.<br></blockquote><div><br></div><div>

Thanks!</div><div><br></div><div>Nick</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<font color="#888888"><br>
-Eli<br>
</font><div><div></div><div class="h5"><br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br>