[PATCH] D141650: [VectorUtils] Enhance VFABI demangling API

Thu Jan 19 01:06:40 PST 2023

fpetrogalli added a subscriber: stuij.
fpetrogalli added a comment.

In D141650#4064127 <https://reviews.llvm.org/D141650#4064127>, @Nuullll wrote:

> In D141650#4056383 <https://reviews.llvm.org/D141650#4056383>, @lebedev.ri wrote:
>
>> In D141650#4055313 <https://reviews.llvm.org/D141650#4055313>, @Nuullll wrote:
>>
>>> Thanks a lot for reviewing!
>>>
>>> @lebedev.ri
>>>
>>>> 1. Do we need that parameter? Is empty <parameters> list generally ill-formed and must be diagnosed?
>>>
>>> There's one subtle difference in AArch64 <https://github.com/ARM-software/abi-aa/blob/2982a9f3b512a5bfdc9e3fea5d3b298f9165c36b/vfabia64/vfabia64.rst#name-mangling-function> and X86 <https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt#CA-a460b8925e5a148d33d8cdc1cace46e03ac88229_98> VFABI documentations about Vector Function Name Mangling: the X86 doc explicitly allows an empty parameter list while the AArch64 doc doesn't -- so I think this is why we need a parameter to let users choose whatever standard to conform. In fact, our internal implementation has to remove the diagnostic on empty parameter from `tryDemangleForVFABI` to make sure we're conforming the X86 VFABI.
>>
>> Nitpick: would it be possible to simply ask AArch64 for clarification, what is the intended behaviour for them?
>
> @fpetrogalli Can you please kindly provide some inputs on AArch64 VFABI? Reference commit: https://github.com/ARM-software/abi-aa/commit/8fcf3f72d7cc200c50a4b029cee295469ab46fe5

It's been a long time since I looked into this, but from a quick skimming thought the Parameter mapping rules <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#41common-rules-for-parameter-mapping> I think the behaviour of AArch64 would be to allow empty parameters list.

If you consider a function with signature `int f(void)`, it means that the function has no input parameters.  This means that the only `<P>` (parameter or return value, in the notation of the specs) considered to create the vector function is the return value.

So, if targeting for example NEON, the sequence of rules for mangling the name would be:

1. `MTV(P) = true` (rule 3 of 4.1.1 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#411maps-to-vector>)
2. `PBV(P) = true` ( rule 1 of 4.1.2 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#412pass-by-value>)
3. `MAP(P) = ADVSIMD_MAP(P)` (rule 2 of 4.1.3 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#413parameter-and-return-value-mapping>)
4. `LS(P) = 4` (rule 2 of 4.2.1 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#lane-size-of-a-function-parameter-return-value> - `T(P)` is the type of `P` in the specs)
5. `NDS(f) = 4` (4.2.2 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#narrowest-and-widest-data-size-of-a-function>)
6. `VLEN = 2,4` (rule 2 of 4.3.1 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#vector-length-advanced-simd>)
7. `ADVSIMD_MAP(P)` is a vector of `VLEN` elements of type `T(P)` (=`int`).  -> `int32x2_t` and `int32x4_t` -> 2 vector functions can be associated to f (because NEON has 64 and 128 bit vector register views) (rule 3 of 4.3.2 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#advanced-simd-specific-rules>)

Therefore, according to the name mangling function at 4.5.1 <https://github.com/ARM-software/abi-aa/blob/main/vfabia64/vfabia64.rst#name-mangling-function>, we have 2 vector functions associated to `f`:

- `int32x2_t _ZVGnN2_f()`
- `int32x4_t _ZVGnN4_f()`

I hope helps clarifying a bit the current situation (unless of course I misinterpreted the specs).

I am CCing @stuij (hello mate! :) ), who I think is responsible for these bits of the compiler and the specs these days.

Notice that in the example with `f` I have ignored the generation of the versions that use Masking, because it seems odd to use masking for a function that have no input parameters... something that would need sorting in the specs, probably?

>>>> 2. I'm not seeing what callee (other than the tests) actually sets that parameter to true?
>>>
>>> Yes, the `tryDemangleForVFABI` API isn't used much in the LLVM codebase (and I didn't touch those usages to keep their behaviors unchanged, for safety). But like I mentioned above, a valid X86 implementation should allow an empty parameter list according to the current VFABI doc.
>>
>> So it's a dead code?
>
> One potential user is @mmasten 's https://reviews.llvm.org/D22792#change-yzkGZ4fTvFrz (see lib/Analysis/VectorVariant.cpp:50).
> And I believe upstreaming VecClone for function vectorization is still part of Intel plan?
> Tagging @xtian

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141650