[VectorUtils] Expose vector-function-abi-variant mangling as a utility. NFC

[anna]

Summary

This is an intended NFC. Cleans code up a bit and also exposes the
vector-function-abi-variant mangling as a utility in VectorUtils.
This is needed for downstream or front-ends that add this
attribute.

[fpetrogalli]

Hi @anna

is this change needed so that you can attach the attribute also to llvm::Function and not just llvm::Callinst? I have explained that this is something we don't want to do: https://reviews.llvm.org/D70107#2034194

Let me know if you have any questions.

Kind regards,

Francesco

[anna]

Hi @fpetrogalli,
Yes, but it should be orthogonal to that requirement. I think hiding the mangling behind a static function, while the demangling is exposed seems unintuitive. Any front-ends that need to attach the attribute will need this function exposed.

[jdoerfert]

I guess exposing this doesn't hurt anyone and allows fronte-ends to reuse this. LG from my perspective.

[anna]

Thanks Johannes. @fpetrogalli Francesco, I believe I've addressed your concern as well?

[fpetrogalli]

[...] but it should be orthogonal to that requirement. I think hiding the mangling behind a static function, while the demangling is exposed seems unintuitive. Any front-ends that need to attach the attribute will need this function exposed.

Yep, I fully agree, thank you for explaining!

The function you are exposing is not a generic VFABI mangling function. The VFABI mangling function would take in input the VFShape that we need to realize from a scalar function, and mangle it accordingly setting the correct tokens for each parameter (whether it is linear, uniform... and so on). The method you are exposing is TLI-specific: all the parameter tokens of the mangled name are rendered with "v". Hence, I think that you should leave the name of the method specific to the TLI, as in VFABI::mangleTLIVectorName. Or alternatively, expose it as a static method of the class TargetLibraryInfo.

Also, given that now this is going to be exposed as method like the demangler, I think it would make sense to add some unit tests in llvm/unittests/Analysis/VectorFunctionABITest.cpp (or the TLI tests if you go for the static method of TLI).

Thank you!

Francesco

Francesco

[simoll]

Make sense to expose this API. Seems ok for me conceptually.
+1 for a test and a nit.

llvm/include/llvm/Analysis/VectorUtils.h

184

Since this is for the LLVM-specific VectorABI, why not rename it to mangleLLVMVectorName for less potential confusion?

[fpetrogalli]

llvm/include/llvm/Analysis/VectorUtils.h

184

I was thinking... given that we are talking about exposing an API that would be needed for all target ISA (including the _LLVM_ one), should we just get it done straight from the beginning? Instead of exposing a TLI only method, implement the following:

std::string VFABI::mangleScalarName(StringRef ScalarName, VFISAKind ISA, VFShape Shape);

This would implement the mangling rules described at https://llvm.org/docs/LangRef.html#call-site-attributes, including unit tests for it.

Then for TLI names, all we need to do would be to invoke it (for example in the frontend, and in the InjectTLIMappings, in a separate patch), as follows:

... = VFABI:mangleScalarName(CI->getCalledFunction()->getName(), VFIsaKind::LLVM, VFShape::get(CI, {VF, false}, false))

@anna , is this something you have time to do? If not, I am fine with exposing the TLI specific method, just make sure that the fact that it is TLI specific is mentioned in the name.

@simoll, regarding:

why not rename it to mangleLLVMVectorName for less potential confusion

I see your point, but the LLVM-specific ABI does not prevent using tokens other than "v", so the method as it is now is really TLI specific, not LLVM-ABI specific.

[anna]

llvm/include/llvm/Analysis/VectorUtils.h

184

I agree that is a good idea @fpetrogalli. Unfortunately, I do not have time for that now. I'll update the patch with the changes you've suggested for the TLI version.

[anna]

addressed review comments. Added unittest case.

[fpetrogalli]

llvm/include/llvm/Analysis/VectorUtils.h

184

Sure, I understand, no worries!

llvm/unittests/Analysis/VectorFunctionABITest.cpp

94–95 ↗

(On Diff #264087)

I don't think we need this. The demangler is already tested extensively. If some tests are missing, I think it is better to add them separately as demangler tests.

I think all we need here (in the body of the test below) is just a couple of invocation of the API that makes sure that the correct string is generated. Something like:

EXPECT_EQ(VFABI::mangleTLIVectorName( "vec", "scalar", 3, 4),"_ZGV_LLVM_N4vvv_scalar(vec)");
EXPECT_EQ(VFABI::mangleTLIVectorName( "A", "B", 4, 5), "_ZGV_LLVM_N5vvvv_B(A)");

[anna]

llvm/unittests/Analysis/VectorFunctionABITest.cpp

94–95 ↗

(On Diff #264087)

okay, I like that idea of decoupling them. Yeah, just checked - we handle all of these in the demangler case.

[anna]

addressed review comment about test case.

[fpetrogalli]

LGTM, with one need that it would be great to fix before submitting to master.

Thank you for working on this!

Francesco

llvm/include/llvm/Analysis/VectorUtils.h

170

Nit: please mention the fact that this method is specific for the TargetLibraryInfo mappings.

[anna]

thank you for the review!

llvm/include/llvm/Analysis/VectorUtils.h

 /// _ZGV<isa><mask><vlen><parameters>_<scalarname>[(<redirection>)].
 /// \param M -> Module used to retrive informations about the vector
 /// function that are not possible to retrieve from the mangled
 /// name. At the moment, this parameter is needed only to retrive the
 /// Vectorization Factor of scalable vector functions from their
 /// respective IR declarations.
 Optional<VFInfo> tryDemangleForVFABI(StringRef MangledName, const Module &M);
 
+/// This routine mangles the given VectorName according to the LangRef
+/// specification for vector-function-abi-variant attribute.
+/// This returned string holds scalar-to-vector mapping:
+///    _ZGV<isa><mask><vlen><vparams>_<scalarname>(<vectorname>)
+///
+/// where:
+///
+/// <isa> = "_LLVM_"
+/// <mask> = "N". Note: TLI does not support masked interfaces.
+/// <vlen> = Number of concurrent lanes, stored in the `VectorizationFactor`
+///          field of the `VecDesc` struct.
+/// <vparams> = "v", as many as are the numArgs.
+/// <scalarname> = the name of the scalar function.
+/// <vectorname> = the name of the vector function.
+std::string mangleVectorName(StringRef VectorName, StringRef ScalarName,
+                             unsigned numArgs, unsigned VF);
+
 /// Retrieve the `VFParamKind` from a string token.
 VFParamKind getVFParamKindFromString(const StringRef Token);
 
 // Name of the attribute where the variant mappings are stored.
 static constexpr char const *MappingsAttrName = "vector-function-abi-variant";
 
 /// Populates a set of strings representing the Vector Function ABI variants
 /// associated to the CallInst CI.

llvm/lib/Analysis/VectorUtils.cpp

 void InterleaveGroup<Instruction>::addMetadata(Instruction *NewInst) const {
   SmallVector<Value *, 4> VL;
   std::transform(Members.begin(), Members.end(), std::back_inserter(VL),
                  [](std::pair<int, Instruction *> p) { return p.second; });
   propagateMetadata(NewInst, VL);
 }
 }
 
+std::string VFABI::mangleVectorName(StringRef VectorName, StringRef ScalarName,
+                                    unsigned numArgs, unsigned VF) {
+  SmallString<256> Buffer;
+  llvm::raw_svector_ostream Out(Buffer);
+  Out << "_ZGV" << VFABI::_LLVM_ << "N" << VF;
+  for (unsigned I = 0; I < numArgs; ++I)
+    Out << "v";
+  Out << "_" << ScalarName << "(" << VectorName << ")";
+  return std::string(Out.str());
+}
+
 void VFABI::getVectorVariantNames(
     const CallInst &CI, SmallVectorImpl<std::string> &VariantMappings) {
   const StringRef S =
       CI.getAttribute(AttributeList::FunctionIndex, VFABI::MappingsAttrName)
           .getValueAsString();
   if (S.empty())
     return;
 

llvm/lib/Transforms/Utils/InjectTLIMappings.cpp

 STATISTIC(NumCallInjected,
           "Number of calls in which the mappings have been injected.");
 
 STATISTIC(NumVFDeclAdded,
           "Number of function declarations that have been added.");
 STATISTIC(NumCompUsedAdded,
           "Number of `@llvm.compiler.used` operands that have been added.");
 
-/// Helper function to map the TLI name to a strings that holds
-/// scalar-to-vector mapping.
-///
-///    _ZGV<isa><mask><vlen><vparams>_<scalarname>(<vectorname>)
-///
-/// where:
-///
-/// <isa> = "_LLVM_"
-/// <mask> = "N". Note: TLI does not support masked interfaces.
-/// <vlen> = Number of concurrent lanes, stored in the `VectorizationFactor`
-///          field of the `VecDesc` struct.
-/// <vparams> = "v", as many as are the number of parameters of CI.
-/// <scalarname> = the name of the scalar function called by CI.
-/// <vectorname> = the name of the vector function mapped by the TLI.
-static std::string mangleTLIName(StringRef VectorName, const CallInst &CI,
-                                 unsigned VF) {
-  SmallString<256> Buffer;
-  llvm::raw_svector_ostream Out(Buffer);
-  Out << "_ZGV" << VFABI::_LLVM_ << "N" << VF;
-  for (unsigned I = 0; I < CI.getNumArgOperands(); ++I)
-    Out << "v";
-  Out << "_" << CI.getCalledFunction()->getName() << "(" << VectorName << ")";
-  return std::string(Out.str());
-}
-
 /// A helper function that adds the vector function declaration that
 /// vectorizes the CallInst CI with a vectorization factor of VF
 /// lanes. The TLI assumes that all parameters and the return type of
 /// CI (other than void) need to be widened to a VectorType of VF
 /// lanes.
 static void addVariantDeclaration(CallInst &CI, const unsigned VF,
                                   const StringRef VFName) {
   Module *M = CI.getModule();
static void addMappingsFromTLI(const TargetLibraryInfo &TLI, CallInst &CI) {
   const SetVector<StringRef> OriginalSetOfMappings(Mappings.begin(),
                                                    Mappings.end());
   //  All VFs in the TLI are powers of 2.
   for (unsigned VF = 2, WidestVF = TLI.getWidestVF(ScalarName); VF <= WidestVF;
        VF *= 2) {
     const std::string TLIName =
         std::string(TLI.getVectorizedFunction(ScalarName, VF));
     if (!TLIName.empty()) {
-      std::string MangledName = mangleTLIName(TLIName, CI, VF);
+      std::string MangledName = VFABI::mangleVectorName(
+          TLIName, ScalarName, CI.getNumArgOperands(), VF);
       if (!OriginalSetOfMappings.count(MangledName)) {
         Mappings.push_back(MangledName);
         ++NumCallInjected;
       }
       Function *VariantF = M->getFunction(TLIName);
       if (!VariantF)
         addVariantDeclaration(CI, VF, TLIName);
     }