[VFABI] Improve VFABI unit tests #73907

paschalis-mpeis · 2023-11-30T08:10:12Z

Do checks for scalar parameter counts for all mappings with matchScalarParametersNum. It is not part of invokeParser so it can be selectively applied only to tests that supply valid mangled names.

Also, minor reformatting and typo fixes.

llvmbot · 2023-11-30T08:10:44Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Paschalis Mpeis (paschalis-mpeis)

Changes

Do checks for scalar parameter counts for all mappings with matchScalarParametersNum. It is not part of invokeParser so it can be selectively applied only to tests that supply valid mangled names.

Also, minor reformatting and typo fixes.

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

1 Files Affected:

(modified) llvm/unittests/Analysis/VectorFunctionABITest.cpp (+105-56)

diff --git a/llvm/unittests/Analysis/VectorFunctionABITest.cpp b/llvm/unittests/Analysis/VectorFunctionABITest.cpp
index e496d87c06de6bc..a95c27ed61d3e18 100644
--- a/llvm/unittests/Analysis/VectorFunctionABITest.cpp
+++ b/llvm/unittests/Analysis/VectorFunctionABITest.cpp
@@ -1,4 +1,4 @@
-//===------- VectorFunctionABITest.cpp - VFABI Unittests  ---------===//
+//===------- VectorFunctionABITest.cpp - VFABI unit tests  ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/InstIterator.h"
@@ -14,7 +15,7 @@
 using namespace llvm;
 
 namespace {
-// Test fixture needed that holds the veariables needed by the parser.
+// Test fixture needed that holds the variables needed by the parser.
 class VFABIParserTest : public ::testing::Test {
 private:
   // Parser output.
@@ -34,6 +35,7 @@ class VFABIParserTest : public ::testing::Test {
         << "The function must be present in the module\n";
     // Reset the VFInfo
     Info = VFInfo();
+    ScalarParametersNum = 0;
   }
 
   // Data needed to load the optional IR passed to invokeParser
@@ -44,10 +46,11 @@ class VFABIParserTest : public ::testing::Test {
   FunctionCallee F;
 
 protected:
-  // Referencies to the parser output field.
+  // References to the parser output field.
   ElementCount &VF = Info.Shape.VF;
   VFISAKind &ISA = Info.ISA;
   SmallVector<VFParameter, 8> &Parameters = Info.Shape.Parameters;
+  size_t ScalarParametersNum;
   std::string &ScalarName = Info.ScalarName;
   std::string &VectorName = Info.VectorName;
   // Invoke the parser. We need to make sure that a function exist in
@@ -78,23 +81,21 @@ class VFABIParserTest : public ::testing::Test {
 
     // Fake the arguments to the CallInst.
     SmallVector<Value *> Args;
-    for (Type *ParamTy : FTy->params()) {
+    for (Type *ParamTy : FTy->params())
       Args.push_back(Constant::getNullValue(ParamTy->getScalarType()));
-    }
     std::unique_ptr<CallInst> CI(CallInst::Create(F, Args));
     const auto OptInfo = VFABI::tryDemangleForVFABI(MangledName, *(CI.get()));
     if (OptInfo) {
       Info = *OptInfo;
+      ScalarParametersNum = Info.Shape.getScalarShape(*CI).Parameters.size();
       return true;
     }
-
     return false;
   }
 
   // Checks that 1. the last Parameter in the Shape is of type
-  // VFParamKind::GlobalPredicate and 2. it is the only one of such
-  // type.
-  bool IsMasked() const {
+  // VFParamKind::GlobalPredicate and 2. it is the only one of such type.
+  bool isMasked() const {
     const auto NGlobalPreds =
         std::count_if(Info.Shape.Parameters.begin(),
                       Info.Shape.Parameters.end(), [](const VFParameter PK) {
@@ -103,6 +104,12 @@ class VFABIParserTest : public ::testing::Test {
     return NGlobalPreds == 1 && Info.Shape.Parameters.back().ParamKind ==
                                     VFParamKind::GlobalPredicate;
   }
+
+  // Checks that the number of vectorized parameters matches the scalar ones.
+  // Takes into account that vectorized calls may also have a Mask.
+  bool matchScalarParametersNum() {
+    return (Parameters.size() - isMasked()) == ScalarParametersNum;
+  }
 };
 } // unnamed namespace
 
@@ -140,8 +147,10 @@ TEST_F(VFABIParserTest, OnlyValidNames) {
 }
 
 TEST_F(VFABIParserTest, ParamListParsing) {
-  EXPECT_TRUE(invokeParser("_ZGVnN2vl16Ls32R3l_foo"));
+  EXPECT_TRUE(invokeParser("_ZGVnN2vl16Ls32R3l_foo", "",
+                           "void(double, i32, i32, ptr, i32)"));
   EXPECT_EQ(Parameters.size(), (unsigned)5);
+  EXPECT_TRUE(matchScalarParametersNum());
   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}));
@@ -168,9 +177,12 @@ TEST_F(VFABIParserTest, ScalarNameAndVectorName_03) {
 }
 
 TEST_F(VFABIParserTest, Parse) {
-  EXPECT_TRUE(invokeParser("_ZGVnN2vls2Ls27Us4Rs5l1L10U100R1000_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVnN2vls2Ls27Us4Rs5l1L10U100R1000_sin", "sin",
+                   "void(double, i32, i32, i32, ptr, i32, i32, i32, ptr)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_FALSE(IsMasked());
+  EXPECT_FALSE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   EXPECT_EQ(Parameters.size(), (unsigned)9);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector, 0}));
@@ -187,9 +199,11 @@ TEST_F(VFABIParserTest, Parse) {
 }
 
 TEST_F(VFABIParserTest, ParseVectorName) {
-  EXPECT_TRUE(invokeParser("_ZGVnN2v_sin(my_v_sin)", "my_v_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVnN2v_sin(my_v_sin)", "my_v_sin", "double(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_FALSE(IsMasked());
+  EXPECT_FALSE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   EXPECT_EQ(Parameters.size(), (unsigned)1);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector, 0}));
@@ -198,9 +212,11 @@ TEST_F(VFABIParserTest, ParseVectorName) {
 }
 
 TEST_F(VFABIParserTest, LinearWithCompileTimeNegativeStep) {
-  EXPECT_TRUE(invokeParser("_ZGVnN2ln1Ln10Un100Rn1000_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVnN2ln1Ln10Un100Rn1000_sin", "",
+                           "double(i32, i32, i32, ptr)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_FALSE(IsMasked());
+  EXPECT_FALSE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   EXPECT_EQ(Parameters.size(), (unsigned)4);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::OMP_Linear, -1}));
@@ -213,17 +229,19 @@ TEST_F(VFABIParserTest, LinearWithCompileTimeNegativeStep) {
 
 TEST_F(VFABIParserTest, ParseScalableSVE) {
   EXPECT_TRUE(invokeParser("_ZGVsMxv_sin(custom_vg)", "sin", "i32(i32)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getScalable(4));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::SVE);
   EXPECT_EQ(ScalarName, "sin");
   EXPECT_EQ(VectorName, "custom_vg");
 }
 
 TEST_F(VFABIParserTest, ParseFixedWidthSVE) {
-  EXPECT_TRUE(invokeParser("_ZGVsM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVsM2v_sin", "", "double(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::SVE);
   EXPECT_EQ(ScalarName, "sin");
   EXPECT_EQ(VectorName, "_ZGVsM2v_sin");
@@ -250,7 +268,9 @@ TEST_F(VFABIParserTest, LinearWithRuntimeStep) {
 }
 
 TEST_F(VFABIParserTest, LinearWithoutCompileTime) {
-  EXPECT_TRUE(invokeParser("_ZGVnN3lLRUlnLnRnUn_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVnN3lLRUlnLnRnUn_sin", "",
+                           "void(i32, i32, ptr, i32, i32, i32, ptr, i32)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(Parameters.size(), (unsigned)8);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::OMP_Linear, 1}));
   EXPECT_EQ(Parameters[1], VFParameter({1, VFParamKind::OMP_LinearVal, 1}));
@@ -300,7 +320,8 @@ TEST_F(VFABIParserTest, InvalidParameter) {
 }
 
 TEST_F(VFABIParserTest, Align) {
-  EXPECT_TRUE(invokeParser("_ZGVsN2l2a2_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVsN2l2a2_sin", "", "void(i32)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(Parameters.size(), (unsigned)1);
   EXPECT_EQ(Parameters[0].Alignment, Align(2));
 
@@ -318,9 +339,10 @@ TEST_F(VFABIParserTest, Align) {
 }
 
 TEST_F(VFABIParserTest, ParseUniform) {
-  EXPECT_TRUE(invokeParser("_ZGVnN2u_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVnN2u_sin", "", "void(i32)"));
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_FALSE(IsMasked());
+  EXPECT_TRUE(matchScalarParametersNum());
+  EXPECT_FALSE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   EXPECT_EQ(Parameters.size(), (unsigned)1);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::OMP_Uniform, 0}));
@@ -350,58 +372,70 @@ TEST_F(VFABIParserTest, ISAIndependentMangling) {
 #define __COMMON_CHECKS                                                        \
   do {                                                                         \
     EXPECT_EQ(VF, ElementCount::getFixed(2));                                  \
-    EXPECT_FALSE(IsMasked());                                                  \
+    EXPECT_FALSE(isMasked());                                                  \
+    EXPECT_TRUE(matchScalarParametersNum());                                   \
     EXPECT_EQ(Parameters.size(), (unsigned)10);                                \
     EXPECT_EQ(Parameters, ExpectedParams);                                     \
     EXPECT_EQ(ScalarName, "sin");                                              \
   } while (0)
 
+  const StringRef IRTy =
+      "void(double, i32, i32, i32, ptr, i32, i32, i32, i32, i32)";
+
   // Advanced SIMD: <isa> = "n"
-  EXPECT_TRUE(invokeParser("_ZGVnN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVnN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVnN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // SVE: <isa> = "s"
-  EXPECT_TRUE(invokeParser("_ZGVsN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVsN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::SVE);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVsN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // SSE: <isa> = "b"
-  EXPECT_TRUE(invokeParser("_ZGVbN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVbN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::SSE);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVbN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // AVX: <isa> = "c"
-  EXPECT_TRUE(invokeParser("_ZGVcN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVcN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::AVX);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVcN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // AVX2: <isa> = "d"
-  EXPECT_TRUE(invokeParser("_ZGVdN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVdN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::AVX2);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVdN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // AVX512: <isa> = "e"
-  EXPECT_TRUE(invokeParser("_ZGVeN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVeN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::AVX512);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVeN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
 
   // LLVM: <isa> = "_LLVM_" internal vector function.
-  EXPECT_TRUE(invokeParser(
-      "_ZGV_LLVM_N2vls2Ls27Us4Rs5l1L10U100R1000u_sin(vectorf)", "vectorf"));
+  EXPECT_TRUE(
+      invokeParser("_ZGV_LLVM_N2vls2Ls27Us4Rs5l1L10U100R1000u_sin(vectorf)",
+                   "vectorf", IRTy));
   EXPECT_EQ(ISA, VFISAKind::LLVM);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "vectorf");
 
   // Unknown ISA (randomly using "q"). This test will need update if
   // some targets decide to use "q" as their ISA token.
-  EXPECT_TRUE(invokeParser("_ZGVqN2vls2Ls27Us4Rs5l1L10U100R1000u_sin"));
+  EXPECT_TRUE(
+      invokeParser("_ZGVqN2vls2Ls27Us4Rs5l1L10U100R1000u_sin", "", IRTy));
   EXPECT_EQ(ISA, VFISAKind::Unknown);
   __COMMON_CHECKS;
   EXPECT_EQ(VectorName, "_ZGVqN2vls2Ls27Us4Rs5l1L10U100R1000u_sin");
@@ -422,9 +456,10 @@ TEST_F(VFABIParserTest, MissingVectorNameTermination) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingNEON) {
-  EXPECT_TRUE(invokeParser("_ZGVnM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVnM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AdvancedSIMD);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -433,9 +468,10 @@ TEST_F(VFABIParserTest, ParseMaskingNEON) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingSVE) {
-  EXPECT_TRUE(invokeParser("_ZGVsM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVsM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::SVE);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -444,9 +480,10 @@ TEST_F(VFABIParserTest, ParseMaskingSVE) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingSSE) {
-  EXPECT_TRUE(invokeParser("_ZGVbM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVbM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::SSE);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -455,9 +492,10 @@ TEST_F(VFABIParserTest, ParseMaskingSSE) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingAVX) {
-  EXPECT_TRUE(invokeParser("_ZGVcM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVcM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AVX);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -466,9 +504,10 @@ TEST_F(VFABIParserTest, ParseMaskingAVX) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingAVX2) {
-  EXPECT_TRUE(invokeParser("_ZGVdM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVdM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AVX2);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -477,9 +516,10 @@ TEST_F(VFABIParserTest, ParseMaskingAVX2) {
 }
 
 TEST_F(VFABIParserTest, ParseMaskingAVX512) {
-  EXPECT_TRUE(invokeParser("_ZGVeM2v_sin"));
+  EXPECT_TRUE(invokeParser("_ZGVeM2v_sin", "", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::AVX512);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -489,9 +529,10 @@ TEST_F(VFABIParserTest, ParseMaskingAVX512) {
 
 TEST_F(VFABIParserTest, ParseMaskingLLVM) {
   EXPECT_TRUE(invokeParser("_ZGV_LLVM_M2v_sin(custom_vector_sin)",
-                           "custom_vector_sin"));
+                           "custom_vector_sin", "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::LLVM);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -503,12 +544,14 @@ TEST_F(VFABIParserTest, ParseMaskingLLVM) {
 TEST_F(VFABIParserTest, ParseScalableMaskingLLVM) {
   EXPECT_FALSE(
       invokeParser("_ZGV_LLVM_Mxv_sin(custom_vector_sin)", "sin", "i32(i32)"));
+  EXPECT_TRUE(matchScalarParametersNum());
 }
 
 TEST_F(VFABIParserTest, ParseScalableMaskingSVE) {
   EXPECT_TRUE(
       invokeParser("_ZGVsMxv_sin(custom_vector_sin)", "sin", "i32(i32)"));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(matchScalarParametersNum());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(VF, ElementCount::getScalable(4));
   EXPECT_EQ(ISA, VFISAKind::SVE);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
@@ -520,9 +563,10 @@ TEST_F(VFABIParserTest, ParseScalableMaskingSVE) {
 
 TEST_F(VFABIParserTest, ParseScalableMaskingSVESincos) {
   EXPECT_TRUE(invokeParser("_ZGVsMxvl8l8_sincos(custom_vector_sincos)",
-                           "sincos", "void(double, double *, double *)"));
+                           "sincos", "void(double, ptr, ptr)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getScalable(2));
-  EXPECT_TRUE(IsMasked());
+  EXPECT_TRUE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::SVE);
   EXPECT_EQ(Parameters.size(), (unsigned)4);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -538,12 +582,14 @@ TEST_F(VFABIParserTest, ParseScalableMaskingSVESincos) {
 TEST_F(VFABIParserTest, ParseWiderReturnTypeSVE) {
   EXPECT_TRUE(
       invokeParser("_ZGVsMxvv_foo(vector_foo)", "foo", "i64(i32, i32)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getScalable(2));
 }
 
 // Make sure we handle void return types.
 TEST_F(VFABIParserTest, ParseVoidReturnTypeSVE) {
   EXPECT_TRUE(invokeParser("_ZGVsMxv_foo(vector_foo)", "foo", "void(i16)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getScalable(8));
 }
 
@@ -556,7 +602,6 @@ TEST_F(VFABIParserTest, ParseUnsupportedElementTypeSVE) {
 TEST_F(VFABIParserTest, ParseUnsupportedReturnTypeSVE) {
   EXPECT_FALSE(invokeParser("_ZGVsMxv_foo(vector_foo)", "foo", "fp128(float)"));
 }
-
 class VFABIAttrTest : public testing::Test {
 protected:
   void SetUp() override {
@@ -593,15 +638,18 @@ TEST_F(VFABIAttrTest, Read) {
 
 TEST_F(VFABIParserTest, LLVM_InternalISA) {
   EXPECT_FALSE(invokeParser("_ZGV_LLVM_N2v_sin"));
-  EXPECT_TRUE(invokeParser("_ZGV_LLVM_N2v_sin_(vector_name)", "vector_name"));
+  EXPECT_TRUE(invokeParser("_ZGV_LLVM_N2v_sin_(vector_name)", "vector_name",
+                           "void(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(ISA, VFISAKind::LLVM);
 }
 
 TEST_F(VFABIParserTest, IntrinsicsInLLVMIsa) {
   EXPECT_TRUE(invokeParser("_ZGV_LLVM_N4vv_llvm.pow.f32(__svml_powf4)",
-                           "__svml_powf4"));
+                           "__svml_powf4", "void(float, float)"));
+  EXPECT_TRUE(matchScalarParametersNum());
   EXPECT_EQ(VF, ElementCount::getFixed(4));
-  EXPECT_FALSE(IsMasked());
+  EXPECT_FALSE(isMasked());
   EXPECT_EQ(ISA, VFISAKind::LLVM);
   EXPECT_EQ(Parameters.size(), (unsigned)2);
   EXPECT_EQ(Parameters[0], VFParameter({0, VFParamKind::Vector}));
@@ -615,6 +663,7 @@ TEST_F(VFABIParserTest, ParseScalableRequiresDeclaration) {
   // `custom_vg` is added to the module.
   EXPECT_FALSE(invokeParser(MangledName));
   EXPECT_TRUE(invokeParser(MangledName, "sin", "double(double)"));
+  EXPECT_TRUE(matchScalarParametersNum());
 }
 
 TEST_F(VFABIParserTest, ZeroIsInvalidVLEN) {
@@ -641,9 +690,9 @@ define void @call(void () * %f) {
   ret void
 }
 )IR");
-  auto F = dyn_cast_or_null<Function>(M->getNamedValue("call"));
+  auto *F = dyn_cast_or_null<Function>(M->getNamedValue("call"));
   ASSERT_TRUE(F);
-  auto CI = dyn_cast<CallInst>(&F->front().front());
+  auto *CI = dyn_cast<CallInst>(&F->front().front());
   ASSERT_TRUE(CI);
   ASSERT_TRUE(CI->isIndirectCall());
   auto Mappings = VFDatabase::getMappings(*CI);

llvm/unittests/Analysis/VectorFunctionABITest.cpp

github-actions · 2023-11-30T18:25:02Z

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

mgabka

I didn't review yet all the test changes.

llvm/unittests/Analysis/VectorFunctionABITest.cpp

llvm/include/llvm/Analysis/VectorUtils.h

llvm/tools/vfabi-demangle-fuzzer/vfabi-demangler-fuzzer.cpp

llvm/unittests/Analysis/VectorFunctionABITest.cpp

Do checks for scalar parameter counts for all mappings with `matchScalarParametersNum`. It is not part of `invokeParser` so it can be selectively applied only to tests that supply valid mangled names. Also, minor reformatting and typo fixes.

`tryDemangleForVFABI` and `VFShape`'s `getScalarShape`, `get` use `FunctionType` instead of `CallInst`. Each successful parsing should provide a vector name, and do checks for the names, number of arguments, and vector parameter types, eg: ``` EXPECT_EQ(ScalarName, "foo"); EXPECT_EQ(VectorName, "vector_foo"); ... EXPECT_TRUE(matchScalarParamNum()); ... EXPECT_EQ(VecFuncParameters[0], VFParameter({0, VFParamKind::OMP_Linear, 2})); ``` Replace method names to `foo` and `vector_foo` for the scalar and vector variants respectively. Using only `i32` for parameters when it's not relevant, except when the VF Elements are explicitly checked, `ptr` for references, and `void` for most return types. All `VFABIParserTest` tests are now listed contiguously in the source.

- Clean up duplicate ISA tests. - Added an extra test to clearly show that the mangled name becomes the VectorName, when no VectorName is specified.

Removed ScalarFuncParametersNum, and now using the ScalarFTy directly, that is parsed from the input ScalarFTyStr.

demangle-fuzzer had an extra parenthesis introduced by previous commit in this PR and it was not formatted properly. In ABI tests, renamed `reset` parameter to match `invokeParser`, and did a few more minor styling changes.

Adjusted code and comments to address reviewers. For masked calls, checking if the last parameter is a mask, only for SVE and NEON ISAs. Reverted VecFuncParameters to Parameters.

Checks in tests are now added in the same order: 1. Verify ISA 2. Verify presence/absence of a mask 3. match the number of parameters Appended the tests if any of those were missing.

llvm/unittests/Analysis/VectorFunctionABITest.cpp

mgabka · 2023-12-10T17:47:04Z

llvm/unittests/Analysis/VectorFunctionABITest.cpp

@@ -423,108 +447,167 @@ TEST_F(VFABIParserTest, MissingVectorNameTermination) {
 }


definitely not for this patch, but at some point would be good to group somehow those tests in this file :)

paschalis-mpeis requested a review from huntergr-arm November 30, 2023 08:10

llvmbot added the llvm:analysis label Nov 30, 2023

paschalis-mpeis requested review from paulwalker-arm and mgabka November 30, 2023 08:10

mgabka reviewed Nov 30, 2023

View reviewed changes

llvm/unittests/Analysis/VectorFunctionABITest.cpp Outdated Show resolved Hide resolved

llvm/unittests/Analysis/VectorFunctionABITest.cpp Show resolved Hide resolved

llvm/unittests/Analysis/VectorFunctionABITest.cpp Outdated Show resolved Hide resolved

llvmbot added vectorization llvm:transforms labels Nov 30, 2023

huntergr-arm reviewed Nov 30, 2023

View reviewed changes

mgabka reviewed Dec 1, 2023

View reviewed changes

llvm/unittests/Analysis/VectorFunctionABITest.cpp Outdated Show resolved Hide resolved

llvm/unittests/Analysis/VectorFunctionABITest.cpp Outdated Show resolved Hide resolved

llvm/unittests/Analysis/VectorFunctionABITest.cpp Outdated Show resolved Hide resolved

paschalis-mpeis marked this pull request as draft December 1, 2023 11:44

paschalis-mpeis force-pushed the vfabi-improve-unit-tests branch 2 times, most recently from 91823c4 to c244744 Compare December 1, 2023 15:24

paschalis-mpeis marked this pull request as ready for review December 1, 2023 15:33

mgabka reviewed Dec 3, 2023

View reviewed changes

paulwalker-arm reviewed Dec 5, 2023

View reviewed changes

paschalis-mpeis marked this pull request as draft December 6, 2023 07:50

paschalis-mpeis force-pushed the vfabi-improve-unit-tests branch from c244744 to 33a6c3e Compare December 6, 2023 10:33

paschalis-mpeis marked this pull request as ready for review December 6, 2023 10:37

paschalis-mpeis force-pushed the vfabi-improve-unit-tests branch from 33a6c3e to e0b4e48 Compare December 6, 2023 18:10

paulwalker-arm reviewed Dec 7, 2023

View reviewed changes

paschalis-mpeis added 8 commits December 8, 2023 09:24

[VFABI] Improve VFABI unit tests

292bbab

Do checks for scalar parameter counts for all mappings with `matchScalarParametersNum`. It is not part of `invokeParser` so it can be selectively applied only to tests that supply valid mangled names. Also, minor reformatting and typo fixes.

Addressing 2nd review round.

cd99514

- Clean up duplicate ISA tests. - Added an extra test to clearly show that the mangled name becomes the VectorName, when no VectorName is specified.

Clang-format issue.

7e4f22b

Made use for Scalar FunctionTypes more clear.

1bcad8b

Removed ScalarFuncParametersNum, and now using the ScalarFTy directly, that is parsed from the input ScalarFTyStr.

Fix for demangle-fuzzer tool and minor refactoring

daef7bc

demangle-fuzzer had an extra parenthesis introduced by previous commit in this PR and it was not formatted properly. In ABI tests, renamed `reset` parameter to match `invokeParser`, and did a few more minor styling changes.

Use VFInfo for isMasked and cleanup

154401f

Adjusted code and comments to address reviewers. For masked calls, checking if the last parameter is a mask, only for SVE and NEON ISAs. Reverted VecFuncParameters to Parameters.

Restored checks for predicates.

4713f95

Checks in tests are now added in the same order: 1. Verify ISA 2. Verify presence/absence of a mask 3. match the number of parameters Appended the tests if any of those were missing.

Updated doxygen

65e202d

paschalis-mpeis force-pushed the vfabi-improve-unit-tests branch from 3446989 to 65e202d Compare December 8, 2023 09:57

paulwalker-arm approved these changes Dec 8, 2023

View reviewed changes

mgabka approved these changes Dec 10, 2023

View reviewed changes

Improved comments.

8fc4f53

paschalis-mpeis merged commit 3c1e7fb into llvm:main Dec 11, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VFABI] Improve VFABI unit tests #73907

[VFABI] Improve VFABI unit tests #73907

paschalis-mpeis commented Nov 30, 2023 •

edited

llvmbot commented Nov 30, 2023 •

edited

github-actions bot commented Nov 30, 2023 •

edited

mgabka left a comment

mgabka Dec 10, 2023

		@@ -423,108 +447,167 @@ TEST_F(VFABIParserTest, MissingVectorNameTermination) {
		}

[VFABI] Improve VFABI unit tests #73907

[VFABI] Improve VFABI unit tests #73907

Conversation

paschalis-mpeis commented Nov 30, 2023 • edited

llvmbot commented Nov 30, 2023 • edited

github-actions bot commented Nov 30, 2023 • edited

mgabka left a comment

Choose a reason for hiding this comment

mgabka Dec 10, 2023

Choose a reason for hiding this comment

paschalis-mpeis commented Nov 30, 2023 •

edited

llvmbot commented Nov 30, 2023 •

edited

github-actions bot commented Nov 30, 2023 •

edited