[PATCH] ARM: Homogeneous aggregates must be allocated to contiguous registers

James Molloy james at jamesmolloy.co.uk
Mon Mar 17 04:54:49 PDT 2014


Hi,

+1 Oliver, in general.

Renato, I agree completely with your end goal. The place I disagree with
you is in the process for achieving that. You seem to be advocating the
"keep piling in more and more hacks until it is sufficiently horrendous
that someone is forced to take the time to go rewrite it from scratch"
methodology, which we both know from prior experience does not actually
work. What actually happens is that the hacks pile up and people back away
slowly and then more quickly from the code, until it is an unmaintained
pile of rubbish. C.f. the clang driver.

We all know that the current design of IR means the frontend must encode
some language-specific information in the IR in some form. Most of this
information is fairly obvious to the reader and innocuous - for example
using an sret* first parameter versus returning an aggregate. Some of this
information is wholly horrible, such as coercing small vectors into i32's
(to hint that it would be better to pass these through GPRs) and making
3-element vectors pass-by-reference instead of by value.

My point is that these decisions - on the contract between frontend and IR
- are on a highly visible external interface and are often not backwards
compatible. We should not be piling hacks into this area because we end up
with technical debt that we have to service for years to come. There are
many out of tree users that construct IR themselves from their own frontend
or manually via C++ or through .ll files, and need that IR to interact with
other functions via the native ABI. Having some things in a contract, such
as sret* versus return-aggregate - is fine and is fairly obvious what the
difference in behaviour is. Reordering arguments certainly is not - it is
unintuitive and would result in a worse mess than the i32-coercion of the
3-element-vector-by-reference debacle that slyly made it into Clang and
made it backwards-incompatible.

So I think Oliver's solution is the best one of the two going forward, and
Anton's is an extra hack that we will have to live with for years - PCS
builder or no PCS builder.

Also Anton, looking at the patch it looks to me like attributes are
*already* 64-bits wide, Oliver is just using two unused bits of it. Unless
I've missed something?

Cheers,

James



On 17 March 2014 10:50, Oliver Stannard <oliver.stannard at arm.com> wrote:

>
>   > I'm in favour of having a PCS helper in the IRBuilder in any form or
> shape, but that's orthogonal to any front-end's inability to produce
> correct ABI code. I wouldn't want a new, generic PCS helper to be moulded
> based on a single ARM HA issue, but to be developed from scratch, with most
> PCSs in mind, including x86, Mips, PPC, AArch64, etc.
>
>   I'm not trying to create a more generic system for handling PCSs, and
> have tried to minimise the amount of code in target-independent places. If
> you have any suggestions to further reduce the target-independent code I
> have added, that would be appreciated.
>
>   > In that sense, I agree with Anton that this *very* specific problem
> sould be fixed in the front-end, as it has historically being done by
> ARM-compatible front-ends (llvm-gcc, clang, our own EDG bridge, and
> others), and a more generic approach should be taken to a wider audience,
> mixing Clang and LLVM developers in a "grand design".
>
>   My understanding of the reason for putting the PCS code in clang is that
> it is required because the LLVM backend cannot handle all types correctly.
> This patch expands the set of types that can be handled by the ARM backend,
> so some of the ARM-specific clang code is no longer necessary. Having two
> separate bits of code allocating arguments to registers has never struck me
> as a particularly robust design.
>
>   > In the end, other front-ends will have to adapt to your own
> implementation's specific details anyway, and it doesn't matter what kind
> of specific behaviour for the front-end engineer, as long as it works. The
> only better scenario is when there is *NO* PCS specific knowledge in a
> function declaration, and all of it is done by the PCS helper. Anything in
> between will be just another shoddy contract.
>
>   This patch strictly increases the set of types that the ARM backends can
> handle without help, so other frontends will continue to work as they
> currently do without changes. To generate ABI-compliant code, they still
> have the option to re-order arguments to get the correct back-filling, but
> this patch allows them to simply emit an argument with struct type, and it
> will be handled correctly.
>
>   I agree that needing no PCS knowledge to create a function definition
> would be ideal, but that would require a major change given that some PCSs
> depend on source-language details that will not always be unambiguously
> represented in IR. However, this does not mean that there is no benefit to
> reducing the amount of PCS knowledge in the frontend, when it could instead
> be handled by the target-specific backend.
>
>   > I do want to see that happening, but not starting from a corner case.
>
>   I would also like to see that happen, but that is not what I am trying
> to do here.
>
> http://llvm-reviews.chandlerc.com/D3082
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140317/27c04131/attachment.html>


More information about the llvm-commits mailing list