[llvm-dev] Aggregate load/stores

deadal nix via llvm-dev llvm-dev at lists.llvm.org
Sun Aug 16 22:27:57 PDT 2015


2015-08-16 22:10 GMT-07:00 David Majnemer <david.majnemer at gmail.com>:
>
>
> I would argue that a fix in the wrong direction is worse than the status
> quo.
>

How is proposed change worse than status quo ?


>
>
>>
>> The argument that target are relying on InstCombine to mitigate IR
>> requiring legalization seems dubious to me. First, because both aggregate
>> and large scalar require legalization, so, if not ideal, the proposed
>> change does not makes things any worse than they already are. In fact, as
>> far as legalization is concerned, theses are pretty much the same. It
>> should also be noted that InstCombine is not guaranteed to run before the
>> target, so it seems like a bad idea to me to rely on it in the backend.
>>
>
> InstCombine is not guaranteed to run before IR hits the backend but the
> result of legalizing the machinations of InstCombine's output during
> SelectionDAG is worse than generating illegal IR in the first place.
>

That does not follow. InstCombine is not creating new things that require
legalisation, it changes one thing that require legalization into another
that a larger part of LLVM can understand.


>
>
>>
>> As for the big integral thing, I really don't care. I can change it to
>> create multiple loads/stores respecting data layout, I have the code for
>> that and could adapt it for this PR without too much trouble. If this is
>> the only thing that is blocking this PR, then we can proceed. But I'd like
>> some notion that we are making progress. Would you be willing to accept a
>> solution based on creating a serie of load/store respecting the datalayout ?
>>
>
> Splitting the memory operation into smaller operations is not semantics
> preserving from an IR-theoretic perspective.  For example, splitting a
> volatile memory operation into several volatile memory operations is not
> OK.  Same goes with atomics.  Some targets provide atomic memory operations
> at the granularity of a cache line and splitting at legal integer
> granularity would be observably different.
>
>
That is off topic. Proposed patch explicitly gate for this.


>
> With the above in mind, I don't see it as unreasonable for frontends to
> generate IR that LLVM is comfortable with.  We seem fine telling frontend
> authors that they should strive to avoid large aggregate memory operations
> in our performance tips guide <
> http://llvm.org/docs/Frontend/PerformanceTips.html#avoid-loads-and-stores-of-large-aggregate-type>.
> Implementation experience with Clang hasn't shown this to be particularly
> odious to follow and none of the LLVM-side solutions seem satisfactory.
>
>
Most front end do not have clang resources. Additionally, this tip is not
quite accurate. I'm not interested in large aggregate load/store at this
stage. I'm interested in ANY aggregate load/store. LLVM is just unable to
handle any of it in a way that make sense. It could certainly do better for
small aggregate, without too much trouble.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150816/715b9089/attachment.html>


More information about the llvm-dev mailing list