From 85f89de6caaa6f6b86593d8057302b95120ca125 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Fri, 5 Jan 2024 15:13:21 +0000 Subject: [PATCH 1/9] Pass replace-with-veclib crashes with invalid arguments --- llvm/unittests/Analysis/CMakeLists.txt | 1 + .../Analysis/ReplaceWithVecLibTest.cpp | 86 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp diff --git a/llvm/unittests/Analysis/CMakeLists.txt b/llvm/unittests/Analysis/CMakeLists.txt index 847430bf17697..e7505f2633d92 100644 --- a/llvm/unittests/Analysis/CMakeLists.txt +++ b/llvm/unittests/Analysis/CMakeLists.txt @@ -40,6 +40,7 @@ set(ANALYSIS_TEST_SOURCES PluginInlineAdvisorAnalysisTest.cpp PluginInlineOrderAnalysisTest.cpp ProfileSummaryInfoTest.cpp + ReplaceWithVecLibTest.cpp ScalarEvolutionTest.cpp VectorFunctionABITest.cpp SparsePropagation.cpp diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp new file mode 100644 index 0000000000000..8f80c67b2ed41 --- /dev/null +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -0,0 +1,86 @@ +//===--- ReplaceWithVecLibTest.cpp - replace-with-veclib 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. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "llvm/CodeGen/ReplaceWithVeclib.h" +#include "llvm/Analysis/TargetLibraryInfo.h" +#include "llvm/AsmParser/Parser.h" +#include "llvm/IR/LLVMContext.h" +#include "llvm/IR/Module.h" +#include "llvm/Passes/PassBuilder.h" +#include "llvm/Support/SourceMgr.h" +#include "gtest/gtest.h" + +using namespace llvm; + +namespace { + +static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { + SMDiagnostic Err; + std::unique_ptr Mod = parseAssemblyString(IR, Err, C); + if (!Mod) + Err.print("ReplaceWithVecLibTest", errs()); + return Mod; +} + +/// Runs ReplaceWithVecLib with different TLIIs that have custom VecDescs. This +/// allows checking that the pass won't crash when the function to replace (from +/// the input IR) does not match the replacement function (derived from the +/// VecDesc mapping). +class ReplaceWithVecLibTest : public ::testing::Test { +protected: + LLVMContext Ctx; + + /// Creates TLII using the given \p VD, and then runs the ReplaceWithVeclib + /// pass. The pass should not crash even when the replacement function + /// (derived from the \p VD mapping) does not match the function to be + /// replaced (from the input \p IR). + bool run(const VecDesc &VD, const char *IR) { + // Create TLII and register it with FAM so it's preserved when + // ReplaceWithVeclib pass runs. + TargetLibraryInfoImpl TLII = TargetLibraryInfoImpl(Triple()); + TLII.addVectorizableFunctions({VD}); + FunctionAnalysisManager FAM; + FAM.registerPass([&TLII]() { return TargetLibraryAnalysis(TLII); }); + + // Register and run the pass on the 'foo' function from the input IR. + FunctionPassManager FPM; + FPM.addPass(ReplaceWithVeclib()); + std::unique_ptr M = parseIR(Ctx, IR); + PassBuilder PB; + PB.registerFunctionAnalyses(FAM); + FPM.run(*M->getFunction("foo"), FAM); + + return true; + } +}; + +} // end anonymous namespace + +static const char *IR = R"IR( +define @foo( %in){ + %call = call @llvm.powi.f32.i32( %in, i32 3) + ret %call +} + +declare @llvm.powi.f32.i32(, i32) #0 +)IR"; + +// LLVM intrinsic 'powi' (in IR) has the same signature with the VecDesc. +TEST_F(ReplaceWithVecLibTest, TestValidMapping) { + VecDesc CorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvu_powi", + ElementCount::getScalable(4), true, "_ZGVsMxvu"}; + EXPECT_TRUE(run(CorrectVD, IR)); +} + +// LLVM intrinsic 'powi' (in IR) has different signature with the VecDesc. +TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { + VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", + ElementCount::getScalable(4), true, "_ZGVsMxvv"}; + /// TODO: test should avoid and not crash. + EXPECT_DEATH(run(IncorrectVD, IR), ""); +} From 9dfc955fa13264e72a85b4dc93c6582cafc486d2 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Fri, 5 Jan 2024 15:25:30 +0000 Subject: [PATCH 2/9] [TLI] Fix replace-with-veclib crashes with invalid arguments. replace-with-veclib used to crash when the arguments of the TLI mapping did not match the arguments of the mapping. Now, it simply ignores such cases. --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 24 +++++++++++++++++-- .../Analysis/ReplaceWithVecLibTest.cpp | 3 +-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index 56025aa5c45fb..92f2d006fd79c 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -108,15 +108,17 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, // Compute the argument types of the corresponding scalar call and the scalar // function name. For calls, it additionally finds the function to replace // and checks that all vector operands match the previously found EC. - SmallVector ScalarArgTypes; + SmallVector ScalarArgTypes, OrigArgTypes; std::string ScalarName; Function *FuncToReplace = nullptr; - if (auto *CI = dyn_cast(&I)) { + auto *CI = dyn_cast(&I); + if (CI) { FuncToReplace = CI->getCalledFunction(); Intrinsic::ID IID = FuncToReplace->getIntrinsicID(); assert(IID != Intrinsic::not_intrinsic && "Not an intrinsic"); for (auto Arg : enumerate(CI->args())) { auto *ArgTy = Arg.value()->getType(); + OrigArgTypes.push_back(ArgTy); if (isVectorIntrinsicWithScalarOpAtArg(IID, Arg.index())) { ScalarArgTypes.push_back(ArgTy); } else if (auto *VectorArgTy = dyn_cast(ArgTy)) { @@ -174,6 +176,24 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, Function *TLIFunc = getTLIFunction(I.getModule(), VectorFTy, VD->getVectorFnName(), FuncToReplace); + + // For calls, bail out when their arguments do not match with the TLI mapping. + if (CI) { + int IdxNonPred = 0; + for (auto [OrigTy, VFParam] : + zip(OrigArgTypes, OptInfo->Shape.Parameters)) { + if (VFParam.ParamKind == VFParamKind::GlobalPredicate) + continue; + ++IdxNonPred; + if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { + LLVM_DEBUG(dbgs() << DEBUG_TYPE + << ": Will not replace: wrong type at index: " + << IdxNonPred << ": " << *OrigTy << "\n"); + return false; + } + } + } + replaceWithTLIFunction(I, *OptInfo, TLIFunc); LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Replaced call to `" << ScalarName << "` with call to `" << TLIFunc->getName() << "`.\n"); diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 8f80c67b2ed41..858f72894861c 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -81,6 +81,5 @@ TEST_F(ReplaceWithVecLibTest, TestValidMapping) { TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", ElementCount::getScalable(4), true, "_ZGVsMxvv"}; - /// TODO: test should avoid and not crash. - EXPECT_DEATH(run(IncorrectVD, IR), ""); + EXPECT_TRUE(run(IncorrectVD, IR)); } From c93f1c097773d8ffb086990a546a517919ad6a54 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Tue, 9 Jan 2024 10:17:16 +0000 Subject: [PATCH 3/9] Using VFParam.ParamPos to access Types --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 27 ++++++++++--------- .../Analysis/ReplaceWithVecLibTest.cpp | 12 ++++++--- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index 92f2d006fd79c..64025bfe031dd 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -170,30 +170,31 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, if (!OptInfo) return false; - FunctionType *VectorFTy = VFABI::createFunctionType(*OptInfo, ScalarFTy); - if (!VectorFTy) - return false; - - Function *TLIFunc = getTLIFunction(I.getModule(), VectorFTy, - VD->getVectorFnName(), FuncToReplace); - - // For calls, bail out when their arguments do not match with the TLI mapping. + // There is no guarantee that the vectorized instructions followed the VFABI + // specification when being created, this is why we need to add extra check to + // make sure that the operands of the vector function obtained via VFABI match + // the operands of the original vector instruction. if (CI) { - int IdxNonPred = 0; - for (auto [OrigTy, VFParam] : - zip(OrigArgTypes, OptInfo->Shape.Parameters)) { + for (auto VFParam : OptInfo->Shape.Parameters) { if (VFParam.ParamKind == VFParamKind::GlobalPredicate) continue; - ++IdxNonPred; + Type *OrigTy = OrigArgTypes[VFParam.ParamPos]; if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Will not replace: wrong type at index: " - << IdxNonPred << ": " << *OrigTy << "\n"); + << VFParam.ParamPos << ": " << *OrigTy << "\n"); return false; } } } + FunctionType *VectorFTy = VFABI::createFunctionType(*OptInfo, ScalarFTy); + if (!VectorFTy) + return false; + + Function *TLIFunc = getTLIFunction(I.getModule(), VectorFTy, + VD->getVectorFnName(), FuncToReplace); + replaceWithTLIFunction(I, *OptInfo, TLIFunc); LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Replaced call to `" << ScalarName << "` with call to `" << TLIFunc->getName() << "`.\n"); diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 858f72894861c..400743af97c8c 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -70,16 +70,20 @@ define @foo( %in){ declare @llvm.powi.f32.i32(, i32) #0 )IR"; -// LLVM intrinsic 'powi' (in IR) has the same signature with the VecDesc. +// The VFABI prefix in TLI describes signature which is matching the powi +// intrinsic declaration. TEST_F(ReplaceWithVecLibTest, TestValidMapping) { VecDesc CorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvu_powi", - ElementCount::getScalable(4), true, "_ZGVsMxvu"}; + ElementCount::getScalable(4), /*Masked*/ true, + "_ZGVsMxvu"}; EXPECT_TRUE(run(CorrectVD, IR)); } -// LLVM intrinsic 'powi' (in IR) has different signature with the VecDesc. +// The VFABI prefix in TLI describes signature which is not matching the powi +// intrinsic declaration. TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", - ElementCount::getScalable(4), true, "_ZGVsMxvv"}; + ElementCount::getScalable(4), /*Masked*/ true, + "_ZGVsMxvv"}; EXPECT_TRUE(run(IncorrectVD, IR)); } From 39cb3532849edc0b46150d45ecc9f10965525ecd Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Tue, 9 Jan 2024 12:47:12 +0000 Subject: [PATCH 4/9] Addressing reviewers --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index 64025bfe031dd..7dccff1bbac74 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -178,6 +178,11 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, for (auto VFParam : OptInfo->Shape.Parameters) { if (VFParam.ParamKind == VFParamKind::GlobalPredicate) continue; + + // tryDemangleForVFABI must return valid ParamPos, otherwise it could be + // a bug in the VFABI parser. + assert(VFParam.ParamPos < OrigArgTypes.size() && + "ParamPos has invalid range."); Type *OrigTy = OrigArgTypes[VFParam.ParamPos]; if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { LLVM_DEBUG(dbgs() << DEBUG_TYPE From 407a68379ac43f8dbf1d77b7fc1e2851084d4fc7 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 10 Jan 2024 12:18:27 +0000 Subject: [PATCH 5/9] Addressing reviewers (2) --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index 7dccff1bbac74..f581cf9fbe9a9 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -108,7 +108,7 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, // Compute the argument types of the corresponding scalar call and the scalar // function name. For calls, it additionally finds the function to replace // and checks that all vector operands match the previously found EC. - SmallVector ScalarArgTypes, OrigArgTypes; + SmallVector ScalarArgTypes; std::string ScalarName; Function *FuncToReplace = nullptr; auto *CI = dyn_cast(&I); @@ -118,7 +118,6 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, assert(IID != Intrinsic::not_intrinsic && "Not an intrinsic"); for (auto Arg : enumerate(CI->args())) { auto *ArgTy = Arg.value()->getType(); - OrigArgTypes.push_back(ArgTy); if (isVectorIntrinsicWithScalarOpAtArg(IID, Arg.index())) { ScalarArgTypes.push_back(ArgTy); } else if (auto *VectorArgTy = dyn_cast(ArgTy)) { @@ -181,9 +180,9 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, // tryDemangleForVFABI must return valid ParamPos, otherwise it could be // a bug in the VFABI parser. - assert(VFParam.ParamPos < OrigArgTypes.size() && + assert(VFParam.ParamPos < CI->arg_size() && "ParamPos has invalid range."); - Type *OrigTy = OrigArgTypes[VFParam.ParamPos]; + Type *OrigTy = CI->getArgOperand(VFParam.ParamPos)->getType(); if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Will not replace: wrong type at index: " From 254025f966132bbfb0b5dcd406a9bd1a4697804a Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Wed, 10 Jan 2024 16:37:22 +0000 Subject: [PATCH 6/9] Improved tests by comparing last line of stderr. --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 5 +++- .../Analysis/ReplaceWithVecLibTest.cpp | 29 +++++++++++++++---- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index f581cf9fbe9a9..7d8686a1681a7 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -185,7 +185,7 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, Type *OrigTy = CI->getArgOperand(VFParam.ParamPos)->getType(); if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { LLVM_DEBUG(dbgs() << DEBUG_TYPE - << ": Will not replace: wrong type at index: " + << ": Will not replace. Wrong type at index " << VFParam.ParamPos << ": " << *OrigTy << "\n"); return false; } @@ -245,6 +245,9 @@ PreservedAnalyses ReplaceWithVeclib::run(Function &F, const TargetLibraryInfo &TLI = AM.getResult(F); auto Changed = runImpl(TLI, F); if (Changed) { + LLVM_DEBUG(dbgs() << "Instructions replaced with vector libraries: " + << NumCallsReplaced << "\n"); + PreservedAnalyses PA; PA.preserveSet(); PA.preserve(); diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 400743af97c8c..2157e5ee0a698 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -32,6 +32,16 @@ static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { /// the input IR) does not match the replacement function (derived from the /// VecDesc mapping). class ReplaceWithVecLibTest : public ::testing::Test { + + std::string getLastLine(std::string Out) { + // remove ending '\n' if it exists + if (!Out.empty() && *(Out.cend() - 1) == '\n') + Out.pop_back(); + + size_t LastNL = Out.find_last_of('\n'); + return (LastNL == std::string::npos) ? Out : Out.substr(LastNL + 1); + } + protected: LLVMContext Ctx; @@ -39,7 +49,10 @@ class ReplaceWithVecLibTest : public ::testing::Test { /// pass. The pass should not crash even when the replacement function /// (derived from the \p VD mapping) does not match the function to be /// replaced (from the input \p IR). - bool run(const VecDesc &VD, const char *IR) { + /// + /// \returns the last line of the standard error to be compared for + /// correctness. + std::string run(const VecDesc &VD, const char *IR) { // Create TLII and register it with FAM so it's preserved when // ReplaceWithVeclib pass runs. TargetLibraryInfoImpl TLII = TargetLibraryInfoImpl(Triple()); @@ -53,9 +66,12 @@ class ReplaceWithVecLibTest : public ::testing::Test { std::unique_ptr M = parseIR(Ctx, IR); PassBuilder PB; PB.registerFunctionAnalyses(FAM); - FPM.run(*M->getFunction("foo"), FAM); - return true; + // Enable debugging and capture std error + llvm::DebugFlag = true; + testing::internal::CaptureStderr(); + FPM.run(*M->getFunction("foo"), FAM); + return getLastLine(testing::internal::GetCapturedStderr()); } }; @@ -76,7 +92,8 @@ TEST_F(ReplaceWithVecLibTest, TestValidMapping) { VecDesc CorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvu_powi", ElementCount::getScalable(4), /*Masked*/ true, "_ZGVsMxvu"}; - EXPECT_TRUE(run(CorrectVD, IR)); + EXPECT_EQ(run(CorrectVD, IR), + "Instructions replaced with vector libraries: 1"); } // The VFABI prefix in TLI describes signature which is not matching the powi @@ -85,5 +102,7 @@ TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", ElementCount::getScalable(4), /*Masked*/ true, "_ZGVsMxvv"}; - EXPECT_TRUE(run(IncorrectVD, IR)); + EXPECT_EQ( + run(IncorrectVD, IR), + "replace-with-veclib: Will not replace. Wrong type at index 1: i32"); } From b4a0b6ef020cd66b20315ba7a36d9483eda37771 Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Thu, 11 Jan 2024 10:07:02 +0000 Subject: [PATCH 7/9] Fix test to work with and without assertions. --- .../Analysis/ReplaceWithVecLibTest.cpp | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 2157e5ee0a698..8905bd9462468 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -34,7 +34,7 @@ static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { class ReplaceWithVecLibTest : public ::testing::Test { std::string getLastLine(std::string Out) { - // remove ending '\n' if it exists + // remove any trailing '\n' if (!Out.empty() && *(Out.cend() - 1) == '\n') Out.pop_back(); @@ -86,6 +86,9 @@ define @foo( %in){ declare @llvm.powi.f32.i32(, i32) #0 )IR"; +// With assertions on, perform stricter checks by verifying the debug output. +#ifndef NDEBUG + // The VFABI prefix in TLI describes signature which is matching the powi // intrinsic declaration. TEST_F(ReplaceWithVecLibTest, TestValidMapping) { @@ -106,3 +109,25 @@ TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { run(IncorrectVD, IR), "replace-with-veclib: Will not replace. Wrong type at index 1: i32"); } + +// Without assertions, check that tests don't crash. +#else + +// The VFABI prefix in TLI describes signature which is matching the powi +// intrinsic declaration. +TEST_F(ReplaceWithVecLibTest, TestValidMapping) { + VecDesc CorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvu_powi", + ElementCount::getScalable(4), /*Masked*/ true, + "_ZGVsMxvu"}; + EXPECT_EQ(run(CorrectVD, IR), ""); +} + +// The VFABI prefix in TLI describes signature which is not matching the powi +// intrinsic declaration. +TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { + VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", + ElementCount::getScalable(4), /*Masked*/ true, + "_ZGVsMxvv"}; + EXPECT_EQ(run(IncorrectVD, IR), ""); +} +#endif \ No newline at end of file From e40d08205ee3f98efd2161e8de9436aab82d8eec Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Thu, 11 Jan 2024 14:06:02 +0000 Subject: [PATCH 8/9] Addressing reviewers. --- llvm/lib/CodeGen/ReplaceWithVeclib.cpp | 6 ++-- .../Analysis/ReplaceWithVecLibTest.cpp | 31 ++++--------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp index 7d8686a1681a7..7b0215535a92c 100644 --- a/llvm/lib/CodeGen/ReplaceWithVeclib.cpp +++ b/llvm/lib/CodeGen/ReplaceWithVeclib.cpp @@ -184,9 +184,9 @@ static bool replaceWithCallToVeclib(const TargetLibraryInfo &TLI, "ParamPos has invalid range."); Type *OrigTy = CI->getArgOperand(VFParam.ParamPos)->getType(); if (OrigTy->isVectorTy() != (VFParam.ParamKind == VFParamKind::Vector)) { - LLVM_DEBUG(dbgs() << DEBUG_TYPE - << ": Will not replace. Wrong type at index " - << VFParam.ParamPos << ": " << *OrigTy << "\n"); + LLVM_DEBUG(dbgs() << DEBUG_TYPE << ": Will not replace: " << ScalarName + << ". Wrong type at index " << VFParam.ParamPos + << ": " << *OrigTy << "\n"); return false; } } diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 8905bd9462468..9447fbc4106e3 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -31,6 +31,8 @@ static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { /// allows checking that the pass won't crash when the function to replace (from /// the input IR) does not match the replacement function (derived from the /// VecDesc mapping). +/// +/// NOTE: Assertions must be enabled for these tests to run. class ReplaceWithVecLibTest : public ::testing::Test { std::string getLastLine(std::string Out) { @@ -86,7 +88,7 @@ define @foo( %in){ declare @llvm.powi.f32.i32(, i32) #0 )IR"; -// With assertions on, perform stricter checks by verifying the debug output. +// Need assertions enabled for running the tests. #ifndef NDEBUG // The VFABI prefix in TLI describes signature which is matching the powi @@ -105,29 +107,8 @@ TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", ElementCount::getScalable(4), /*Masked*/ true, "_ZGVsMxvv"}; - EXPECT_EQ( - run(IncorrectVD, IR), - "replace-with-veclib: Will not replace. Wrong type at index 1: i32"); -} - -// Without assertions, check that tests don't crash. -#else - -// The VFABI prefix in TLI describes signature which is matching the powi -// intrinsic declaration. -TEST_F(ReplaceWithVecLibTest, TestValidMapping) { - VecDesc CorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvu_powi", - ElementCount::getScalable(4), /*Masked*/ true, - "_ZGVsMxvu"}; - EXPECT_EQ(run(CorrectVD, IR), ""); -} - -// The VFABI prefix in TLI describes signature which is not matching the powi -// intrinsic declaration. -TEST_F(ReplaceWithVecLibTest, TestInvalidMapping) { - VecDesc IncorrectVD = {"llvm.powi.f32.i32", "_ZGVsMxvv_powi", - ElementCount::getScalable(4), /*Masked*/ true, - "_ZGVsMxvv"}; - EXPECT_EQ(run(IncorrectVD, IR), ""); + EXPECT_EQ(run(IncorrectVD, IR), + "replace-with-veclib: Will not replace: llvm.powi.f32.i32. Wrong " + "type at index 1: i32"); } #endif \ No newline at end of file From 86380ab4e38f7308dd3a38b0ccd8fed2918e827d Mon Sep 17 00:00:00 2001 From: Paschalis Mpeis Date: Thu, 11 Jan 2024 17:41:13 +0000 Subject: [PATCH 9/9] Addressing reviewers (2) --- llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp index 9447fbc4106e3..a1f0a4a894c8d 100644 --- a/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp +++ b/llvm/unittests/Analysis/ReplaceWithVecLibTest.cpp @@ -17,6 +17,9 @@ using namespace llvm; +/// NOTE: Assertions must be enabled for these tests to run. +#ifndef NDEBUG + namespace { static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { @@ -32,7 +35,6 @@ static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { /// the input IR) does not match the replacement function (derived from the /// VecDesc mapping). /// -/// NOTE: Assertions must be enabled for these tests to run. class ReplaceWithVecLibTest : public ::testing::Test { std::string getLastLine(std::string Out) { @@ -88,9 +90,6 @@ define @foo( %in){ declare @llvm.powi.f32.i32(, i32) #0 )IR"; -// Need assertions enabled for running the tests. -#ifndef NDEBUG - // The VFABI prefix in TLI describes signature which is matching the powi // intrinsic declaration. TEST_F(ReplaceWithVecLibTest, TestValidMapping) {