[LLVMdev] ReduceLoadWidth, DAGCombiner and non 8bit loads/extloads question.

[Ryan Taylor]

These LD1 extloads are generated independent of ReduceLoadWidth (ie by
other DAGCombiner functions) so shouldReduceLoadWidth alone does not solve
this issue it seems?

On Tue, Mar 3, 2015 at 4:32 PM, Ryan Taylor <ryta1203 at gmail.com> wrote:

> Ahmed,
>
>   It looks like shouldReduceLoadWidth is the best solution, since we can
> create override and just return false if the new size is 8, this should
> avoid generating LD1 in this case. I'm also seeing LD1 being generated by
> other opt functions in DAGCombiner, so I'm not sure this is a full solution
> for us, unless they are also generating LD1 via ReduceLoadWidth, I'm not
> sure yet.
>
>   Though we are not yet using the version of LLVM that has the
> shouldReduceLoadWidth virtual function.
>
> On Tue, Mar 3, 2015 at 2:08 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
> wrote:
>
>> On Tue, Mar 3, 2015 at 10:35 AM, Ryan Taylor <ryta1203 at gmail.com> wrote:
>> > I'm curious about this code in ReduceLoadWidth (and in DAGCombiner in
>> > general):
>> >
>> > if (LegalOperations && !TLI.isLoadExtLegal(ExtType, ExtVT))
>> >    return SDValue();
>> >
>> > LegalOperations is false for the first pre-legalize pass and true for
>> the
>> > post-legalize pass. The first pass is target-independent yes? So that
>> makes
>> > sense.
>> >
>> > The issue we are having is this: we don't support 8 bit loads and we
>> don't
>> > support 8 bit extloads, so we end up with LD1 with zext after either the
>> > first pass or the second pass (depending on the test case). If we add
>> the
>> > TargetLowering callback method setLoadExtAction(ISD::ZEXTLOAD, MVT::i8,
>> > Expand) then it crashes during legalization
>>
>> This part is surprising.  What happens?  This seems to me like the
>> correct solution.
>>
>> I'm guessing it's because there's no good way to legalize 8 bit loads?
>>  I have no idea what happens when those are illegal, as I'd expect
>> them to be always available?
>>
>>
>> Here's a cheap hack though: I see that ReduceLoadWidth is predicated on
>> the
>> TLI "shouldReduceLoadWidth" hook.  Did you try overriding that to
>> avoid creating 8 bit loads?
>>
>> > and if we don't have that in
>> > then it crashes during instruction selection.
>> >
>> > There are two ways to fix this:
>> >
>> > 1) Add the setLoadExtAction AND comment out 'LegalOperations &&' in the
>> > conditional. (this solves the problem)
>> >
>> > 2) Create a custom expand to undo the optimization added by
>> ReduceLoadWidth.
>> >
>> > The 2nd approach seems more in line with what LLVM infrastructure wants
>> but
>> > it seems silly to have to undo an optimization?
>> >
>> > Essentially, we have some bit packing structures and the code is trying
>> to
>> > get the upper bits. The initial dag generates an LD2 with srl (which
>> makes
>> > sense, it's what we want). The DAGCombiner then goes in and changes
>> that LD2
>> > with srl to an LD1 zextload, which we don't support.
>> >
>> > Why is LegalOperations really needed here? What is the purpose and
>> point of
>> > this? It seems you could eliminate this and be all the better for it.
>>
>> FWIW I somewhat agree, and believe this is a common "problem":  we
>> eagerly generate obviously-illegal nodes, because we're before
>> legalisation, so it's OK, right?  Except, it's sometimes hard to
>> recover from.  Most of the time, it's a good thing, precisely because
>> it catches patterns that would be disturbed by legalization.
>>
>> Did you try running the integration tests - at least X86 - after
>> changing the condition?
>>
>> -Ahmed
>>
>> > Thanks.
>> >
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150304/010f6f2e/attachment.html>