llvm dev - Mar 2021 - Controlling parameter alignment

To double check, the problem is the last argument here
https://godbolt.org/z/Gqva4Y does not carry the align(16)
information anymore, correct?

If so, I don't see an immediate problem with allowing an
"alignment"
be applied to non-pointer values. The semantics would probably say
that if the value is demoted to memory/stack, the associated alignment
has to be guaranteed. As noted before [0], `align` is actually a hint
and not a requirement. I'm unsure if we could not make it binding TBH,
will think about that, interested in opinions.

I take it we can't/don't want to legalize the constraints in the
backend, right? I mean, use `%sturct.S* align(16) byval(%sturct.S) %p`
in the IR. On that thought, what happens if we have a `struct S *`
argument in source which we then promote to pass it by value?

~ Johannes


[0] https://reviews.llvm.org/D75903#2549569


On 2/25/21 4:21 PM, Reid Kleckner wrote:
> +Johannes, are there other folks who care about LLVM IR attributes that we
> can add?
>
> WDYT about trying to separate actual argument memory alignment from pointer
> argument alignment?
>
> On Wed, Feb 17, 2021 at 4:24 AM Momchil Velikov <momchil.velikov at
gmail.com>
> wrote:
>
>> Currently we don't have a way to express in LLVM IR the alignment
>> requirements of the function arguments. The `align` attribute is
>> applicable to pointers only, and only for some special ways of passing
>> arguments (e..g `byval`). When implementing AAPCS32/AAPCS64, clang
>> resorts to dubious hacks of coercing to types, which naturally have
>> the needed alignment.
>>
>> But we don't have enough types to cover all the cases.
>>
>> The problem appeared when passing over-aligned Homogeneous
>> Floating-Point Aggregates (HFAs). When we pass a type with increased
>> alignment requirements, for example
>>
>> struct S {
>>    __attribute__ ((__aligned__(16))) double v[4];
>> };
>>
>> Clang uses `[4 x double]` for the parameter, which is passed on the
stack
>> at alignment 8, whereas it should be at alignment 16.
>>
>> The trick of coercing to, say, `[i128, i128]` does not work, because
>> the argument may end up in GP registers. A hypothetical coercion to
>> `[f128, f128]` won't work either, because argument needs to be
>> expanded to consecutive FP registers and they aren't overlapping on
>> AArch64 (e.g. Q1 does not overlap D2/D3).
>>
>> There was a deficiency in the ABI
>> which is addressed in a proposed fix
>> (https://github.com/ARM-software/abi-aa/pull/67), which matches GCC
>> behaviour and the original intent.
>>
>> With this ABI fix, we would need alignments of 8 and 16 to pass HFA
>> arguments, although we should be ideally looking at a generic
>> solution.
>>
>> There are similar issues with AAPCS32.
>>
>> One proposal was to adopt the `stackalign` attribute to convey these
>> alignment requirement, https://reviews.llvm.org/D75903.
>>
>> Another option is to extend the `align` attribute semantics to apply
>> to non-pointer arguments (I have a patch for that, which looks very
>> much as the one above).
>>
>> To which Reid Kleckner commented like:
>>
>>> Mostly I think I meant that this will be a big change in the
meaning
>>> of either the align or the alignstack attributes, and that should
be
>>> hashed out on llvm-dev.
>>>
>>> Right now align is kind of a hybrid between an optimization
annotation
>>> attribute, like dereferenceable or nonnull, and an ABI attribute,
like
>>> byval or inreg. When align is used with byval, it affects argument
>>> memory layout. When byval is not present, it is just an optimizer
>>> hint. IMO, ideally, we should separate those two roles.
>>>
>>> I should be able to control the alignment of the memory used to
pass a
>>> pointer argument, at the same time that I annotate which low bits
of
>>> the pointer are known to be zero.
>> Thoughts?
>>
>> ~chill
>> --
>> Compiler scrub, Arm
>>

On Thu, Feb 25, 2021 at 5:00 PM Doerfert, Johannes <jdoerfert at anl.gov>
wrote:
> To double check, the problem is the last argument here
> https://godbolt.org/z/Gqva4Y does not carry the align(16)
> information anymore, correct?
>
Basically, yes.

> If so, I don't see an immediate problem with allowing an
"alignment"
> be applied to non-pointer values. The semantics would probably say
> that if the value is demoted to memory/stack, the associated alignment
> has to be guaranteed. As noted before [0], `align` is actually a hint
> and not a requirement. I'm unsure if we could not make it binding TBH,
> will think about that, interested in opinions.
>
> I take it we can't/don't want to legalize the constraints in the
> backend, right? I mean, use `%sturct.S* align(16) byval(%sturct.S) %p`
> in the IR. On that thought, what happens if we have a `struct S *`
> argument in source which we then promote to pass it by value?
>
Yes, we wouldn't want to use byval in the frontend because it is best to
use IR arguments that are lowered correctly regardless of their position in
the argument list. Using byval usually forces the argument to be passed in
memory, and that isn't correct if the six double arguments are removed from
the original example. This has the surprising consequence that we can't,
for example, remove arguments from vararg functions:
https://reviews.llvm.org/rGe6e88f99b35f2fde5008e36f0d93f5231564b7d8
 And, generally, byval results in worse code at the call site than passing
SSA values does.

If LLVM is able to do argument promotion, usually that means there is no
ABI compatibility concern, and the arguments don't have to be aligned or
arranged in any particular way. I think argument promotion in this case
would pass each field individually.

----

I think what we need here is a new attribute, or a repurposed attribute, to
control argument alignment. The original patch uses alignstack(n) for this
purpose, which to date is used to control the stack pointer alignment for
the whole call frame, and not for arguments. Do people like that, or is it
confusing? If it's OK to use alignstack(n) on an argument, should we start
using it on byval arguments in addition to align(), or is that just churn?
Is it useful to have this kind of strong separation between ABI attributes
and optimization hint attributes?
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210226/d93157c7/attachment.html>

On 2/26/21 2:50 PM, Reid Kleckner wrote:
> On Thu, Feb 25, 2021 at 5:00 PM Doerfert, Johannes <jdoerfert at
anl.gov>
> wrote:
>
>> To double check, the problem is the last argument here
>> https://godbolt.org/z/Gqva4Y does not carry the align(16)
>> information anymore, correct?
>>
> Basically, yes.
>
>
>> If so, I don't see an immediate problem with allowing an
"alignment"
>> be applied to non-pointer values. The semantics would probably say
>> that if the value is demoted to memory/stack, the associated alignment
>> has to be guaranteed. As noted before [0], `align` is actually a hint
>> and not a requirement. I'm unsure if we could not make it binding
TBH,
>> will think about that, interested in opinions.
>>
>> I take it we can't/don't want to legalize the constraints in
the
>> backend, right? I mean, use `%sturct.S* align(16) byval(%sturct.S) %p`
>> in the IR. On that thought, what happens if we have a `struct S *`
>> argument in source which we then promote to pass it by value?
>>
> Yes, we wouldn't want to use byval in the frontend because it is best
to
> use IR arguments that are lowered correctly regardless of their position in
> the argument list. Using byval usually forces the argument to be passed in
> memory, and that isn't correct if the six double arguments are removed
from
> the original example. This has the surprising consequence that we
can't,
> for example, remove arguments from vararg functions:
> https://reviews.llvm.org/rGe6e88f99b35f2fde5008e36f0d93f5231564b7d8
>   And, generally, byval results in worse code at the call site than passing
> SSA values does.
>
> If LLVM is able to do argument promotion, usually that means there is no
> ABI compatibility concern, and the arguments don't have to be aligned
or
> arranged in any particular way. I think argument promotion in this case
> would pass each field individually.
Right, the [4xdouble] should not be exposed as first class value by arg 
promotion.

>
> ----
>
> I think what we need here is a new attribute, or a repurposed attribute, to
> control argument alignment. The original patch uses alignstack(n) for this
> purpose, which to date is used to control the stack pointer alignment for
> the whole call frame, and not for arguments. Do people like that, or is it
> confusing? If it's OK to use alignstack(n) on an argument, should we
start
> using it on byval arguments in addition to align(), or is that just churn?
> Is it useful to have this kind of strong separation between ABI attributes
> and optimization hint attributes?
>
alignstack seems OK to me. We can't (easily) reuse align because it can 
be dropped elsewhere and we might not want to have special rules.

Using alignstack for byval would also eliminate the special rule (for 
align) there I guess.

~ Johannes