<div dir="ltr"><div>Thanks for the reply:</div><div><br></div><div>So should LLVM continue to assume 8-bit byte addressing? It would be nice, not only to us but potential future machines, to have a permanent fix to this assumption? This sounds reasonable yes?</div><div><br></div><div>Marking them as Custom in XXXISelLowering still produces error, the pre-legalize phase is still going to opt to LD1 since it's not caring about target-specifics. Then, again, we'll be stuck trying to undo the addressing. </div><div><br></div><div>I see what you mean about the DAGCombiner, that's a two part solution, first turning them into target-specific before any opts in pre-legalize and then back before any opts in post-legalize. Certainly a potential solution, though we may lose some optimization opportunities, even on valid loads, as you mention.</div><div><br></div><div>The ultimate issue here, for us, is that LLVM makes the assumption that every machine is going to support 8-bit byte-addressing. I'm not sure this is a solid or even reasonable assumption going forward? </div><div><br></div><div>Possibly, we could just pseudo support it and expand it later, but the addressing would still be an issue, I believe.</div><div><br></div><div>Thanks.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 5, 2015 at 2:09 PM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Wed, Mar 4, 2015 at 11:43 AM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>> wrote:<br>
> Ahmed,<br>
><br>
>  Yes, we do not have an 8 bit type and do not support 8 bit loads/extloads.<br>
><br>
>   For your first post, I imagine that anything that the DAGCombiner does it<br>
> could undo EXCEPT deciding to opt to a type that is not allowed,<br>
<br>
</span>No, I think the SelectionDAG legalization should be able to "undo" any<br>
illegal type as well.  For loads/stores, this usually means making the<br>
memory size bigger.  For instance, an i4 store would be legalized to<br>
use i8, and that's what getStoreSize is for.  LLVM assumes 8bit<br>
byte-addressing though, so that's fine.  Again, you can't really<br>
change the number of bytes addressed though, so you can't do much<br>
here.<br>
<span><br>
> but that's<br>
> the design choice made by LLVM: to allow illegal operations AND types during<br>
> the pre-legalize phase, yes<br>
<br>
</span>In general, yes.<br>
<span><br>
> So this is either a bug (design flaw) or maybe<br>
> someone more knowledgeable about a way to get around this without having to<br>
> change core code that cannot be put into off. release will chime in. :)<br>
><br>
>  Yes, via Expand the assert is issued during LegalizeLoadOps (since it's<br>
> trying to legalize the 8 bit extload because the hardware does not support<br>
> it).<br>
><br>
>  The EXT part is not bugging me, I suppose it's possible that the<br>
> DAGCombiner could optimize to just an 8-bit load, but we've only seen this<br>
> opt if it's going to an extload, this is why I keep mentioning it. 8 bit<br>
> loads (NON_EXTLOAD) is also illegal on our machine. We do not support any<br>
> type of load that is 8 bit. Does LLVM also feel that 8 bit NON_EXTLOAD<br>
> should always be supported also? For example, the Legalize can expand an<br>
> EXTLOAD to NON_EXTLOAD and EXT but that would not help either, since we<br>
> don't support 8 bit loads.<br>
><br>
>  I would just like to know the cleanest possible solution to this issue<br>
> within LLVM infrastructure, assuming it's possible with the current 'core'<br>
> code?<br>
<br>
</span>Well, I can come up with a bunch of hacks to avoid messing with upstream code =)<br>
<br>
If you have an answer to the "how do you legalize 8-bit loads?"<br>
problem I mentioned (perhaps MMOs provide enough information), then<br>
the easiest way would be to just do so in your target.  Perhaps a<br>
target-specific DAGCombine would work, or marking loads as "Custom".<br>
<br>
Another solution would be to immediately DAGCombine (again,<br>
target-specific code, which IIRC run before target-independent<br>
DAGCombines) loads into target-specific loads.  This means you'd<br>
sacrifice some of the target-independent combines on ISD::LOAD, but<br>
you can avoid that by turning your target-specific XXXISD::LOAD nodes<br>
into target-independent counterparts in another DAGCombine, right<br>
after legalization.<br>
<br>
You can also play whack-a-mole with such DAGCombines (there isn't that<br>
much of them on LOAD/STOREs, other operations should be easy to<br>
promote/expand) and submit patches;  I don't think we can justify<br>
keeping them as is if we clearly don't support the result.<br>
<span class="HOEnZb"><font color="#888888"><br>
-Ahmed<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> Thanks.<br>
><br>
> On Wed, Mar 4, 2015 at 2:20 PM, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>><br>
> wrote:<br>
>><br>
>> On Wed, Mar 4, 2015 at 10:49 AM, Ahmed Bougacha<br>
>> <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>> wrote:<br>
>> > On Wed, Mar 4, 2015 at 10:26 AM, Ryan Taylor <<a href="mailto:ryta1203@gmail.com">ryta1203@gmail.com</a>> wrote:<br>
>> >>  Yes, it is breaking during the legalize phase, depending on which<br>
>> >> TargetLowering callback method we use. For example, Custom will let it<br>
>> >> through to instructions selection, which it breaks at the that phase,<br>
>> >> otherwise I believe it breaks during legalization. If we use Expand<br>
>> >> instead,<br>
>> >> the assert during Legalize is: "EXTLOAD should always be supported". I<br>
>> >> don't<br>
>> >> really understand that message :)<br>
>> ><br>
>> > Keep in mind "EXTLOAD" usually means "load, possibly followed by an<br>
>> > extension".  So, the "EXT" part is probably irrelevant here, if that's<br>
>> > what's bugging you ;)<br>
>><br>
>> Nevermind, grepping around shows this is specifically about<br>
>> ISD::EXTLOAD, in LegalizeLoadOps (LegalizeDAG.cpp).<br>
>><br>
>> There's some code above, with an "isTypeLegal(SrcVT)" check, that<br>
>> tries to turn an EXTLOAD into LOAD+[SZ]EXT.  I'm guessing that on your<br>
>> target, both the EXTLOAD from i8 and the i8 type are illegal?<br>
>><br>
>> In that case, again, I don't know how one could legalize this.<br>
>><br>
>> -Ahmed<br>
><br>
><br>
</div></div></blockquote></div><br></div>