[InjectTLIMappings] Remove signext/zeroext attributes from vector functions.

[yetingk]

Previously, when declaring vector functions, its attributes are copied by its scalar function. It may be illegal, since signext/zeroext could not be used for vector types.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Yeting Kuo (yetingk)

Changes

Previously, when declaring vector functions, its attributes are copied by its scalar function. It may be illegal, since signext/zeroext could not be used for vector types.

---

Full diff: https://github.com/llvm/llvm-project/pull/80546.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/InjectTLIMappings.cpp (+15)

diff --git a/llvm/lib/Transforms/Utils/InjectTLIMappings.cpp b/llvm/lib/Transforms/Utils/InjectTLIMappings.cpp
index 9bfac2ac9167e..27bfee4dfc7a9 100644
--- a/llvm/lib/Transforms/Utils/InjectTLIMappings.cpp
+++ b/llvm/lib/Transforms/Utils/InjectTLIMappings.cpp
@@ -34,6 +34,19 @@ STATISTIC(NumVFDeclAdded,
 STATISTIC(NumCompUsedAdded,
           "Number of `@llvm.compiler.used` operands that have been added.");
 
+static void removeIllegalAttributes(Function *Func, const FunctionType *FTy) {
+  // Vector types could not have attributes signext/zeroext.
+  if (FTy->getReturnType()->isVectorTy()) {
+    Func->removeRetAttr(Attribute::SExt);
+    Func->removeRetAttr(Attribute::ZExt);
+  }
+  for (const auto I : enumerate(FTy->params()))
+    if (I.value()->isVectorTy()) {
+      Func->removeParamAttr(I.index(), Attribute::SExt);
+      Func->removeParamAttr(I.index(), Attribute::ZExt);
+    }
+}
+
 /// A helper function that adds the vector variant declaration for vectorizing
 /// the CallInst \p CI with a vectorization factor of \p VF lanes. For each
 /// mapping, TLI provides a VABI prefix, which contains all information required
@@ -56,6 +69,8 @@ static void addVariantDeclaration(CallInst &CI, const ElementCount &VF,
   Function *VecFunc =
       Function::Create(VectorFTy, Function::ExternalLinkage, VFName, M);
   VecFunc->copyAttributesFrom(CI.getCalledFunction());
+  removeIllegalAttributes(VecFunc, VectorFTy);
+
   ++NumVFDeclAdded;
   LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Added to the module: `" << VFName
                     << "` of type " << *VectorFTy << "\n");

[yetingk]

Ping.

[topperc]

could not -> cannot

[yetingk]

Done.

[arcbbb]

Is there a test for this change?

[yetingk]

Is there a test for this change?

Actually no, but there were bugs related to it in sifive internal.

[david-arm]

Is it safe to just always remove the attributes if doing so would lead to incorrect results? Do the vector result and arguments now need explicit sign or zero-extending?

[yetingk]

signext and zeroext attribute is illegal to vector attributes. It will cause ICE like Attribute 'signext' applied to incompatible type!.

[david-arm]

Sure, I understand. But then is it safe to even use a vector mapping in that case? I'm just asking if perhaps this is the wrong fix, and really we shouldn't be vectorising at all?

[yetingk]

I think signext/zeroext is to specify the behavior of a data is narrower than its storage. For most vector/simd implement, their scalar datas are packed into its vector register, so we don't need signext/zeroext for them. And I think it's the reason that I don't see an issue to adding signext/zeroext attributes for vector types.

[yetingk]

Ping.