[VFABI] Reject demangled variants with unexpected number of params. #76855
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When demangling a vector variant we are not checking that the number of parameters is the same as that of the scalar function. This check is hoisted out of getScalableECFromSignature() making the equvalent check in the unittests obsolete.
llvmbot
added
the
llvm:analysis
|
@llvm/pr-subscribers-llvm-analysis Author: Alexandros Lamprineas (labrinea) ChangesWhen demangling a vector variant we are not checking that the number of parameters is the same as that of the scalar function. This check is hoisted out of getScalableECFromSignature() making the equvalent check in the unittests obsolete. Full diff: https://github.com/llvm/llvm-project/pull/76855.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/VFABIDemangling.cpp b/llvm/lib/Analysis/VFABIDemangling.cpp
index 426f98c0c6284a..8562d8fbfa1ede 100644
--- a/llvm/lib/Analysis/VFABIDemangling.cpp
+++ b/llvm/lib/Analysis/VFABIDemangling.cpp
@@ -326,10 +326,6 @@ getScalableECFromSignature(const FunctionType *Signature, const VFISAKind ISA,
// Only vector parameters are used when determining the VF; uniform or
// linear are left as scalars, so do not affect VF.
if (Param.ParamKind == VFParamKind::Vector) {
- // If the scalar function doesn't actually have a corresponding argument,
- // reject the mapping.
- if (Param.ParamPos >= Signature->getNumParams())
- return std::nullopt;
Type *PTy = Signature->getParamType(Param.ParamPos);
std::optional<ElementCount> EC = getElementCountForTy(ISA, PTy);
@@ -427,6 +423,11 @@ std::optional<VFInfo> VFABI::tryDemangleForVFABI(StringRef MangledName,
if (Parameters.empty())
return std::nullopt;
+ // If the number of arguments of the scalar function does not match the
+ // vector variant we have just demangled then reject the mapping.
+ if (Parameters.size() != FTy->getNumParams())
+ return std::nullopt;
+
// Figure out the number of lanes in vectors for this function variant. This
// is easy for fixed length, as the vlen encoding just gives us the value
// directly. However, if the vlen mangling indicated that this function
diff --git a/llvm/unittests/Analysis/VectorFunctionABITest.cpp b/llvm/unittests/Analysis/VectorFunctionABITest.cpp
index b72b4b3b21d439..d8a7d8245bb0f3 100644
--- a/llvm/unittests/Analysis/VectorFunctionABITest.cpp
+++ b/llvm/unittests/Analysis/VectorFunctionABITest.cpp
@@ -88,14 +88,6 @@ class VFABIParserTest : public ::testing::Test {
/// Returns whether the parsed function contains a mask.
bool isMasked() const { return Info.isMasked(); }
- /// Check if the number of vectorized parameters matches the scalar ones. This
- /// requires a correct scalar FunctionType string to be fed to the
- /// 'invokeParser'. Mask parameters that are only required by the vector
- /// function call are ignored.
- bool matchParametersNum() {
- return (Parameters.size() - isMasked()) == ScalarFTy->getNumParams();
- }
-
FunctionType *getFunctionType() {
return VFABI::createFunctionType(Info, ScalarFTy);
}
@@ -162,7 +154,6 @@ TEST_F(VFABIParserTest, ParamListParsing) {
invokeParser("_ZGVnN2vl16Ls32R3l_foo", "void(i32, i32, i32, ptr, i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_EQ(false, isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{VectorType::get(Type::getInt32Ty(Ctx), ElementCount::getFixed(2)),
@@ -184,7 +175,6 @@ TEST_F(VFABIParserTest, ScalarNameAndVectorName_01) {
EXPECT_TRUE(invokeParser("_ZGVnM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_EQ(true, isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(ScalarName, "foo");
EXPECT_EQ(VectorName, "vector_foo");
@@ -194,7 +184,6 @@ TEST_F(VFABIParserTest, ScalarNameAndVectorName_02) {
EXPECT_TRUE(invokeParser("_ZGVnM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_EQ(true, isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(ScalarName, "foo");
EXPECT_EQ(VectorName, "vector_foo");
@@ -205,14 +194,13 @@ TEST_F(VFABIParserTest, ScalarNameAndVectorName_03) {
invokeParser("_ZGVnM2v___foo_bar_abc(fooBarAbcVec)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_EQ(true, isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(ScalarName, "__foo_bar_abc");
EXPECT_EQ(VectorName, "fooBarAbcVec");
}
TEST_F(VFABIParserTest, ScalarNameOnly) {
- EXPECT_TRUE(invokeParser("_ZGVnM2v___foo_bar_abc"));
+ EXPECT_TRUE(invokeParser("_ZGVnM2v___foo_bar_abc", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_EQ(true, isMasked());
EXPECT_EQ(ScalarName, "__foo_bar_abc");
@@ -227,7 +215,6 @@ TEST_F(VFABIParserTest, Parse) {
"void(i32, i32, i32, i32, ptr, i32, i32, i32, ptr)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{
@@ -262,7 +249,6 @@ TEST_F(VFABIParserTest, ParseVectorName) {
EXPECT_TRUE(invokeParser("_ZGVnN2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyNoMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)1);
@@ -276,7 +262,6 @@ TEST_F(VFABIParserTest, LinearWithCompileTimeNegativeStep) {
"void(i32, i32, i32, ptr)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{Type::getInt32Ty(Ctx), Type::getInt32Ty(Ctx), Type::getInt32Ty(Ctx),
@@ -297,7 +282,6 @@ TEST_F(VFABIParserTest, ParseScalableSVE) {
EXPECT_TRUE(invokeParser("_ZGVsMxv_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskedVLA_i32);
EXPECT_EQ(VF, ElementCount::getScalable(4));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -311,7 +295,6 @@ TEST_F(VFABIParserTest, ParseFixedWidthSVE) {
EXPECT_TRUE(invokeParser("_ZGVsM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -329,16 +312,16 @@ TEST_F(VFABIParserTest, NotAVectorFunctionABIName) {
TEST_F(VFABIParserTest, LinearWithRuntimeStep) {
EXPECT_FALSE(invokeParser("_ZGVnN2ls_foo"))
<< "A number should be present after \"ls\".";
- EXPECT_TRUE(invokeParser("_ZGVnN2ls2_foo"));
+ EXPECT_TRUE(invokeParser("_ZGVnN2ls2_foo", "void(i32)"));
EXPECT_FALSE(invokeParser("_ZGVnN2Rs_foo"))
<< "A number should be present after \"Rs\".";
- EXPECT_TRUE(invokeParser("_ZGVnN2Rs4_foo"));
+ EXPECT_TRUE(invokeParser("_ZGVnN2Rs4_foo", "void(i32)"));
EXPECT_FALSE(invokeParser("_ZGVnN2Ls_foo"))
<< "A number should be present after \"Ls\".";
- EXPECT_TRUE(invokeParser("_ZGVnN2Ls6_foo"));
+ EXPECT_TRUE(invokeParser("_ZGVnN2Ls6_foo", "void(i32)"));
EXPECT_FALSE(invokeParser("_ZGVnN2Us_foo"))
<< "A number should be present after \"Us\".";
- EXPECT_TRUE(invokeParser("_ZGVnN2Us8_foo"));
+ EXPECT_TRUE(invokeParser("_ZGVnN2Us8_foo", "void(i32)"));
}
TEST_F(VFABIParserTest, LinearWithoutCompileTime) {
@@ -346,7 +329,6 @@ TEST_F(VFABIParserTest, LinearWithoutCompileTime) {
"void(i32, i32, ptr, i32, i32, i32, ptr, i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{Type::getInt32Ty(Ctx), Type::getInt32Ty(Ctx),
@@ -373,7 +355,6 @@ TEST_F(VFABIParserTest, LLVM_ISA) {
EXPECT_TRUE(invokeParser("_ZGV_LLVM_N2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::LLVM);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyNoMaskVLen2_i32);
EXPECT_EQ(Parameters.size(), (unsigned)1);
EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -393,7 +374,6 @@ TEST_F(VFABIParserTest, Align) {
EXPECT_TRUE(invokeParser("_ZGVsN2l2a2_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(Parameters.size(), (unsigned)1);
EXPECT_EQ(Parameters[0].Alignment, Align(2));
EXPECT_EQ(ScalarName, "foo");
@@ -409,7 +389,7 @@ TEST_F(VFABIParserTest, Align) {
EXPECT_FALSE(invokeParser("_ZGVsM2a2_foo"));
// Alignment must be a power of 2.
EXPECT_FALSE(invokeParser("_ZGVsN2l2a0_foo"));
- EXPECT_TRUE(invokeParser("_ZGVsN2l2a1_foo"));
+ EXPECT_TRUE(invokeParser("_ZGVsN2l2a1_foo", "void(i32)"));
EXPECT_FALSE(invokeParser("_ZGVsN2l2a3_foo"));
EXPECT_FALSE(invokeParser("_ZGVsN2l2a6_foo"));
}
@@ -418,7 +398,6 @@ TEST_F(VFABIParserTest, ParseUniform) {
EXPECT_TRUE(invokeParser("_ZGVnN2u_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy =
FunctionType::get(Type::getVoidTy(Ctx), {Type::getInt32Ty(Ctx)}, false);
EXPECT_EQ(getFunctionType(), FTy);
@@ -463,7 +442,6 @@ TEST_F(VFABIParserTest, ISAIndependentMangling) {
do { \
EXPECT_EQ(VF, ElementCount::getFixed(2)); \
EXPECT_FALSE(isMasked()); \
- EXPECT_TRUE(matchParametersNum()); \
EXPECT_EQ(getFunctionType(), FTy); \
EXPECT_EQ(Parameters.size(), (unsigned)10); \
EXPECT_EQ(Parameters, ExpectedParams); \
@@ -539,7 +517,6 @@ TEST_F(VFABIParserTest, ParseMaskingNEON) {
EXPECT_TRUE(invokeParser("_ZGVnM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -553,7 +530,6 @@ TEST_F(VFABIParserTest, ParseMaskingSVE) {
EXPECT_TRUE(invokeParser("_ZGVsM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -567,7 +543,6 @@ TEST_F(VFABIParserTest, ParseMaskingSSE) {
EXPECT_TRUE(invokeParser("_ZGVbM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SSE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -581,7 +556,6 @@ TEST_F(VFABIParserTest, ParseMaskingAVX) {
EXPECT_TRUE(invokeParser("_ZGVcM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AVX);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -595,7 +569,6 @@ TEST_F(VFABIParserTest, ParseMaskingAVX2) {
EXPECT_TRUE(invokeParser("_ZGVdM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AVX2);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -609,7 +582,6 @@ TEST_F(VFABIParserTest, ParseMaskingAVX512) {
EXPECT_TRUE(invokeParser("_ZGVeM2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::AVX512);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -623,7 +595,6 @@ TEST_F(VFABIParserTest, ParseMaskingLLVM) {
EXPECT_TRUE(invokeParser("_ZGV_LLVM_M2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::LLVM);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskVLen2_i32);
EXPECT_EQ(VF, ElementCount::getFixed(2));
EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -642,7 +613,6 @@ TEST_F(VFABIParserTest, LLVM_InternalISA) {
EXPECT_TRUE(invokeParser("_ZGV_LLVM_N2v_foo(vector_foo)", "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::LLVM);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyNoMaskVLen2_i32);
EXPECT_EQ(Parameters.size(), (unsigned)1);
EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -655,7 +625,6 @@ TEST_F(VFABIParserTest, LLVM_Intrinsics) {
"void(float, float)"));
EXPECT_EQ(ISA, VFISAKind::LLVM);
EXPECT_FALSE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{
@@ -678,7 +647,6 @@ TEST_F(VFABIParserTest, ParseScalableRequiresDeclaration) {
EXPECT_TRUE(invokeParser(MangledName, "void(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
EXPECT_EQ(getFunctionType(), FTyMaskedVLA_i32);
EXPECT_EQ(Parameters.size(), (unsigned)2);
EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -698,7 +666,6 @@ TEST_F(VFABIParserTest, ParseScalableMaskingSVE) {
EXPECT_TRUE(invokeParser("_ZGVsMxv_foo(vector_foo)", "i32(i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
VectorType::get(Type::getInt32Ty(Ctx), ElementCount::getScalable(4)),
{VectorType::get(Type::getInt32Ty(Ctx), ElementCount::getScalable(4)),
@@ -718,7 +685,6 @@ TEST_F(VFABIParserTest, ParseScalableMaskingSVESincos) {
"void(double, ptr, ptr)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{
@@ -745,7 +711,6 @@ TEST_F(VFABIParserTest, ParseWiderReturnTypeSVE) {
EXPECT_TRUE(invokeParser("_ZGVsMxvv_foo(vector_foo)", "i64(i32, i32)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
VectorType::get(Type::getInt64Ty(Ctx), ElementCount::getScalable(2)),
{
@@ -769,7 +734,6 @@ TEST_F(VFABIParserTest, ParseVoidReturnTypeSVE) {
EXPECT_TRUE(invokeParser("_ZGVsMxv_foo(vector_foo)", "void(i16)"));
EXPECT_EQ(ISA, VFISAKind::SVE);
EXPECT_TRUE(isMasked());
- EXPECT_TRUE(matchParametersNum());
FunctionType *FTy = FunctionType::get(
Type::getVoidTy(Ctx),
{
|
paschalis-mpeis
approved these changes
Jan 5, 2024
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
…lvm#76855) When demangling a vector variant we are not checking that the number of parameters is the same as that of the scalar function. This check is hoisted out of getScalableECFromSignature() making the equvalent check in the unittests obsolete.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When demangling a vector variant we are not checking that the number of parameters is the same as that of the scalar function. This check is hoisted out of getScalableECFromSignature() making the equvalent check in the unittests obsolete.