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

Wed Jan 18 19:08:54 PST 2023

Nuullll added subscribers: xtian, mmasten.
Nuullll added a comment.

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

>>> 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