[PATCH] D141409: [SystemZ] Fix handling of vectors and their exposure of the vector ABI.

[Andreas Krebbel via Phabricator via cfe-commits]

Andreas-Krebbel added a comment.

In D141409#4083192 <https://reviews.llvm.org/D141409#4083192>, @uweigand wrote:

> In D141409#4083017 <https://reviews.llvm.org/D141409#4083017>, @jonpa wrote:
>
>> Review addressed - hopefully this time with a better result.
>>
>> I think the patch now does what I want it to, while I found some test cases while checking against GCC which I am a little unsure of:
>>
>> (using GCC 12.2.1, 20220901)
>
> @Andreas-Krebbel any comments on this?
>
>> GCC does *not* emit a gnu attribute for this (A wide vector should have different alignments):
>>
>>   typedef __attribute__((vector_size(32))) int v8i32;
>>   void GlobFun(v8i32 (*f)(v8i32));
>>   static v8i32 foo(v8i32 Arg) {
>>     return Arg;
>>   }
>>   void fun() {
>>     GlobFun(foo);
>>   }
>
> This is somewhat unclear - these are passed via "hidden" pointers that cannot be discovered by user code, and I'm not sure the alignment requirements for "normal" pointer apply to those.  The current ABI doc says:
>
>> Replace such an argument by a pointer to the object, or to a copy where necessary to enforce call-by-value semantics.
>
> While not explicitly stated, it would indeed be reasonable to read into that an alignment requirement, i.e. that the pointer has the same requirement as a "normal" pointer to the object.  But in the new ABI, that requirement is explicitly stated as 8 bytes.
>
> The old behavior is not explicitly specified by any ABI doc, so we can really only go after what GCC actually used to do.   And in fact GCC used to (and still does!) simply place such arguments on the stack and passes a hidden pointer to the stack slot - without any dynamic stack realignment, so it in fact only guarantees 8 byte alignment.  So I think it is *safe* to assume that using 8 bytes always is OK - and in that sense it is safe to omit the GNU attribute.
>
> On the other hand, LLVM does actually align these to 32 bytes, so for LLVM the vector support feature does actually constitute a change in the de-facto ABI.   (Still, LLVM-generated code doesn't appear to *rely* on the extended alignment in compiler-generated code anywhere - and these pointers are not discoverable by user code, so alignment cannot make any difference there either.   So it should still be safe.)
>
>> GCC *does* emit a gnu attribute for this (Since the arguments are passed via address and the vectors are only 8 bytes, there should be no visible ABI, or?):
>>
>>   typedef __attribute__((vector_size(8))) int v2i32;
>>   void bar(v2i32 VArr[4], v2i32 *Dst) { *Dst = VArr[3]; } 
>
> I agree, this seems unnecessary.
>
>> GCC does *not* emit it for this, but I don't understand why as the vectors are 32 bytes in size (with v4i32 the attribute is emitted as I expected):
>>
>>   typedef __attribute__((vector_size(32))) int v8i32;
>>   void bar(v8i32 VArr[2], v8i32 *Dst) { *Dst = VArr[1]; }
>
> That seems a bug, the difference in alignment is definitely visible to user code here.

Indeed, the routine doing the checks in GCC does not appear handle vector arguments any different if they are explicitly passed as pointers. This cannot be correct. I'll have a look.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141409/new/

https://reviews.llvm.org/D141409