[VectorUtils] Enhance VFABI demangling API

[Nuullll]

Summary

This patch adds more flexibility to the original VFABI demangling API.

1. Allow demangling an empty parameter list, which is explicitly allowed by X86 VFABI doc (section 2.6) and probably implicitly allowed by AArch64 VFABI doc (section 4.1) as well.
2. Allow passing the module as a nullptr, so that we don't check the existence of the demangled vector function name. This is useful when we want to create the widened function according to the demangling result.

Signed-off-by: Yilong Guo <[e-mail]>

[lebedev.ri]

Not really familiar with this.

1. Do we need that parameter? Is empty <parameters> list generally ill-formed and must be diagnosed?
2. I'm not seeing what callee (other than the tests) actually sets that parameter to true?

(marking as reviewed)

[Nuullll]

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 and X86 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.

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.

[lebedev.ri]

In 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 and X86 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?

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?

[Nuullll]

In D141650#4056383, @lebedev.ri wrote:

In 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 and X86 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

[fpetrogalli]

In D141650#4064127, @Nuullll wrote:

In D141650#4056383, @lebedev.ri wrote:

In 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 and X86 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 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)
2. PBV(P) = true ( rule 1 of 4.1.2)
3. MAP(P) = ADVSIMD_MAP(P) (rule 2 of 4.1.3)
4. LS(P) = 4 (rule 2 of 4.2.1 - T(P) is the type of P in the specs)
5. NDS(f) = 4 (4.2.2)
6. VLEN = 2,4 (rule 2 of 4.3.1)
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)

Therefore, according to the name mangling function at 4.5.1, 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

[stuij]

In D141650#4064617, @fpetrogalli wrote:

In D141650#4064127, @Nuullll wrote:

In D141650#4056383, @lebedev.ri wrote:

In 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 and X86 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 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)
2. PBV(P) = true ( rule 1 of 4.1.2)
3. MAP(P) = ADVSIMD_MAP(P) (rule 2 of 4.1.3)
4. LS(P) = 4 (rule 2 of 4.2.1 - T(P) is the type of P in the specs)
5. NDS(f) = 4 (4.2.2)
6. VLEN = 2,4 (rule 2 of 4.3.1)
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)

Therefore, according to the name mangling function at 4.5.1, 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?

Hi @rsandifo-arm :) As our VFABI document owner, your thoughts?

[rsandifo-arm]

I'm just responding to the above without having full context, so sorry if this is rehashing old discussions.

I agree with @fpetrogalli (hi!) that the intention probably wasn't to exclude parameterless functions. I suppose we should extend the grammar rule:

<parameters> := <parameter> { <parameter> }

to:

<parameters> := [<parameter> <parameters>]

But we don't have the equivalent of X86's:

d) If none of the above three cases is applicable, the CDT is int.

And I think that might have been deliberate. The X86 definition means that:

#pragma omp declare simd
void f(void);

is valid and produces multiple masked and unmasked versions (confirmed with GCC, whose implementation I'm more familiar with). But it isn't clear to me how such a function would be used, or how one would decide which vector ISA and vlen is the best fit. If there are well-defined semantics for this then perhaps we should change the AArch64 definition to allow it. But if there aren't well-defined semantics for it then it probably makes sense to keep things as they are.

So if it's just a question of whether the AArch64 ABI allows some parameterless functions, then like @fpetrogalli says, I think it does/should. But as it stands, there are some combinations that can be successfully demangled, but would never actually result from a valid #pragma. That includes the combination of a void return and no parameter types.

[Nuullll]

Thanks all for the inputs!

In D141650#4084163, @rsandifo-arm wrote:

So if it's just a question of whether the AArch64 ABI allows some parameterless functions, then like @fpetrogalli says, I think it does/should. But as it stands, there are some combinations that can be successfully demangled, but would never actually result from a valid #pragma. That includes the combination of a void return and no parameter types.

I think we are in agreement that we should allow demangling a paramterless function name for both AArch64 and X86 VFABIs. I'll update the revision.

I agree with @fpetrogalli (hi!) that the intention probably wasn't to exclude parameterless functions. I suppose we should extend the grammar rule:

<parameters> := <parameter> { <parameter> }

to:

<parameters> := [<parameter> <parameters>]

@rsandifo-arm Will you kindly help clarify this in the AArch64 VFABI document?

[Nuullll]

Removed the AllowEmptyParameter parameter.

[Nuullll]

A gentle ping :-)

[Nuullll]

ping #2

[mmasten]

LGTM. Are there any other outstanding concerns from anyone? From what I can tell from the latest comments everyone is ok with this change.

[Nuullll]

Hello, can anyone help commit this patch? Thanks!

[Nuullll]

ping #3

llvm/include/llvm/Analysis/VectorUtils.h

 /// _ZGV<isa><mask><vlen><parameters>_<scalarname>[(<redirection>)].
 /// \param M -> Module used to retrieve informations about the vector
 /// function that are not possible to retrieve from the mangled
 /// name. At the moment, this parameter is needed only to retrieve the
 /// Vectorization Factor of scalable vector functions from their
 /// respective IR declarations.
 std::optional<VFInfo> tryDemangleForVFABI(StringRef MangledName,
                                           const Module &M);
+/// \param M -> The pointer to module used to retrieve informations about the
+/// vector function. If nullptr is passed, we don't check the existence of
+/// the demangled vector function.
+std::optional<VFInfo> tryDemangleForVFABI(StringRef MangledName,
+                                          const Module *M = nullptr);
+VFInfo demangleForVFABI(StringRef MangledName);
 
 /// This routine mangles the given VectorName according to the LangRef
 /// specification for vector-function-abi-variant attribute and is specific to
 /// the TLI mappings. It is the responsibility of the caller to make sure that
 /// this is only used if all parameters in the vector function are vector type.
 /// This returned string holds scalar-to-vector mapping:
 ///    _ZGV<isa><mask><vlen><vparams>_<scalarname>(<vectorname>)
 ///

llvm/lib/Analysis/VFABIDemangling.cpp

ElementCount getECFromSignature(FunctionType *Signature) {
   for (auto *Ty : Signature->params())
     if (auto *VTy = dyn_cast<VectorType>(Ty))
       return VTy->getElementCount();
 
   return ElementCount::getFixed(/*Min=*/1);
 }
 } // namespace
 
+VFInfo VFABI::demangleForVFABI(StringRef MangledName) {
+  auto VI = VFABI::tryDemangleForVFABI(MangledName, /*M = */ nullptr);
+  assert(VI && "Invalid name for a VFABI variant.");
+  return VI.value();
+}
+
+std::optional<VFInfo> VFABI::tryDemangleForVFABI(StringRef MangledName,
+                                                 const Module &M) {
+  return VFABI::tryDemangleForVFABI(MangledName, &M);
+}
+
 // Format of the ABI name:
 // _ZGV<isa><mask><vlen><parameters>_<scalarname>[(<redirection>)]
 std::optional<VFInfo> VFABI::tryDemangleForVFABI(StringRef MangledName,
-                                                 const Module &M) {
+                                                 const Module *M) {
   const StringRef OriginalName = MangledName;
   // Assume there is no custom name <redirection>, and therefore the
   // vector name consists of
   // _ZGV<isa><mask><vlen><parameters>_<scalarname>.
   StringRef VectorName = MangledName;
 
   // Parse the fixed size part of the manled name
   if (!MangledName.consume_front("_ZGV"))
if (ParamFound == ParseRet::OK) {
       if (AlignFound == ParseRet::Error)
         return std::nullopt;
 
       // Add the parameter.
       Parameters.push_back({ParameterPos, PKind, StepOrPos, Alignment});
     }
   } while (ParamFound == ParseRet::OK);
 
-  // A valid MangledName must have at least one valid entry in the
-  // <parameters>.
-  if (Parameters.empty())
-    return std::nullopt;
-
   // Check for the <scalarname> and the optional <redirection>, which
   // are separated from the prefix with "_"
   if (!MangledName.consume_front("_"))
     return std::nullopt;
 
   // The rest of the string must be in the format:
   // <scalarname>[(<redirection>)]
   const StringRef ScalarName =
std::optional<VFInfo> VFABI::tryDemangleForVFABI(StringRef MangledName,
   // Adjust the VF for scalable signatures. The EC.Min is not encoded
   // in the name of the function, but it is encoded in the IR
   // signature of the function. We need to extract this information
   // because it is needed by the loop vectorizer, which reasons in
   // terms of VectorizationFactor or ElementCount. In particular, we
   // need to make sure that the VF field of the VFShape class is never
   // set to 0.
   if (IsScalable) {
-    const Function *F = M.getFunction(VectorName);
+    assert(M && "Can't demangle scalable variant name without a valid module!");
+    const Function *F = M->getFunction(VectorName);
     // The declaration of the function must be present in the module
     // to be able to retrieve its signature.
     if (!F)
       return std::nullopt;
     const ElementCount EC = getECFromSignature(F->getFunctionType());
     VF = EC.getKnownMinValue();
   }
 
   // 1. We don't accept a zero lanes vectorization factor.
-  // 2. We don't accept the demangling if the vector function is not
-  // present in the module.
+  // 2. If M is not nullptr, we don't accept the demangling if the vector
+  // function is not present in the module.
   if (VF == 0)
     return std::nullopt;
-  if (!M.getFunction(VectorName))
+  if (M && !M->getFunction(VectorName))
     return std::nullopt;
 
   const VFShape Shape({ElementCount::get(VF, IsScalable), Parameters});
   return VFInfo({Shape, std::string(ScalarName), std::string(VectorName), ISA});
 }
 
 VFParamKind VFABI::getVFParamKindFromString(const StringRef Token) {
   const VFParamKind ParamKind = StringSwitch<VFParamKind>(Token)

llvm/unittests/Analysis/VectorFunctionABITest.cpp

protected:
   //
   // \p IRType -> FunctionType string to be used for the signature of
   // the vector function.  The correct signature is needed by the
   // parser only for scalable functions. For the sake of testing, the
   // generic fixed-length case can use as signature `void()`.
   //
   bool invokeParser(const StringRef MangledName,
                     const StringRef VectorName = "",
-                    const StringRef IRType = "void()") {
+                    const StringRef IRType = "void()",
+                    bool CheckVectorNameExistence = true) {
     StringRef Name = MangledName;
     if (!VectorName.empty())
       Name = VectorName;
     // Reset the VFInfo and the Module to be able to invoke
     // `invokeParser` multiple times in the same test.
     reset(Name, IRType);
 
-    const auto OptInfo = VFABI::tryDemangleForVFABI(MangledName, *(M.get()));
+    const auto OptInfo = VFABI::tryDemangleForVFABI(
+        MangledName, CheckVectorNameExistence ? M.get() : nullptr);
     if (OptInfo) {
       Info = *OptInfo;
       return true;
     }
 
     return false;
   }
 
TEST_F(VFABIParserTest, OnlyValidNames) {
   // Incomplete string.
   EXPECT_FALSE(invokeParser(""));
   EXPECT_FALSE(invokeParser("_ZGV"));
   EXPECT_FALSE(invokeParser("_ZGVn"));
   EXPECT_FALSE(invokeParser("_ZGVnN"));
   EXPECT_FALSE(invokeParser("_ZGVnN2"));
   EXPECT_FALSE(invokeParser("_ZGVnN2v"));
   EXPECT_FALSE(invokeParser("_ZGVnN2v_"));
-  // Missing parameters.
-  EXPECT_FALSE(invokeParser("_ZGVnN2_foo"));
   // Missing _ZGV prefix.
   EXPECT_FALSE(invokeParser("_ZVnN2v_foo"));
   // Missing <isa>.
   EXPECT_FALSE(invokeParser("_ZGVN2v_foo"));
   // Missing <mask>.
   EXPECT_FALSE(invokeParser("_ZGVn2v_foo"));
   // Missing <vlen>.
   EXPECT_FALSE(invokeParser("_ZGVnNv_foo"));
   // Missing <scalarname>.
   EXPECT_FALSE(invokeParser("_ZGVnN2v_"));
   // Missing _ separator.
   EXPECT_FALSE(invokeParser("_ZGVnN2vfoo"));
   // Missing <vectorname>. Using `fakename` because the string being
   // parsed is not a valid function name that `invokeParser` can add.
   EXPECT_FALSE(invokeParser("_ZGVnN2v_foo()", "fakename"));
   // Unterminated name. Using `fakename` because the string being
   // parsed is not a valid function name that `invokeParser` can add.
   EXPECT_FALSE(invokeParser("_ZGVnN2v_foo(bar", "fakename"));
 }
 
+TEST_F(VFABIParserTest, EmptyParamList) {
+  EXPECT_TRUE(invokeParser("_ZGVnN2_foo", /*VectorName = */ "",
+                           /*IRType = */ "void()"));
+}
+
+TEST_F(VFABIParserTest, DontCheckVectorNameExistence) {
+  // Demangling succeeds even if `UserFunc` doesn't exist in the module.
+  EXPECT_TRUE(invokeParser("_ZGVnM2v_sin(UserFunc)", /*VectorName = */ "",
+                           /*IRType = */ "void()",
+                           /*CheckVectorNameExistence = */ false));
+  EXPECT_EQ(ScalarName, "sin");
+  EXPECT_EQ(VectorName, "UserFunc");
+}
+
 TEST_F(VFABIParserTest, ParamListParsing) {
   EXPECT_TRUE(invokeParser("_ZGVnN2vl16Ls32R3l_foo"));
   EXPECT_EQ(Parameters.size(), (unsigned)5);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector, 0}));
   EXPECT_EQ(Parameters[1], VFParameter({1, VFParamKind::OMP_Linear, 16}));
   EXPECT_EQ(Parameters[2], VFParameter({2, VFParamKind::OMP_LinearValPos, 32}));
   EXPECT_EQ(Parameters[3], VFParameter({3, VFParamKind::OMP_LinearRef, 3}));
   EXPECT_EQ(Parameters[4], VFParameter({4, VFParamKind::OMP_Linear, 1}));