diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h index 9576b935198dd..1d72d395c9f8b 100644 --- a/llvm/include/llvm/AsmParser/LLParser.h +++ b/llvm/include/llvm/AsmParser/LLParser.h @@ -171,6 +171,10 @@ namespace llvm { std::map>> ForwardRefTypeIds; + // Locations for all call instructions that call an overloaded intrinsic + // (for accurate error reporting). + std::map OverloadedIntrinsicCallLocs; + // Map of module ID to path. std::map ModuleIdMap; diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h index 866c68d15e401..3d56c9086ef1a 100644 --- a/llvm/include/llvm/IR/Function.h +++ b/llvm/include/llvm/IR/Function.h @@ -269,7 +269,12 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node { /// getIntrinsicID() returns Intrinsic::not_intrinsic. bool isConstrainedFPIntrinsic() const; - static Intrinsic::ID lookupIntrinsicID(StringRef Name); + static std::pair + lookupIntrinsicIDAndSuffix(StringRef Name); + + static Intrinsic::ID lookupIntrinsicID(StringRef Name) { + return lookupIntrinsicIDAndSuffix(Name).first; + } /// Update internal caches that depend on the function name (such as the /// intrinsic ID and libcall cache). diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 66edf1e5a2957..e3a062d330301 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -338,7 +338,7 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) { for (const auto &[Name, Info] : make_early_inc_range(ForwardRefVals)) { if (StringRef(Name).starts_with("llvm.")) { - Intrinsic::ID IID = Function::lookupIntrinsicID(Name); + auto [IID, Suffix] = Function::lookupIntrinsicIDAndSuffix(Name); if (IID == Intrinsic::not_intrinsic) // Don't do anything for unknown intrinsics. continue; @@ -349,18 +349,37 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) { // // Additionally, automatically add the required mangling suffix to the // intrinsic name. This means that we may replace a single forward - // declaration with multiple functions here. + // declaration with multiple functions here. If there is a suffix left + // over after matching the intrinsic name, it should match the mangling + // suffix. for (Use &U : make_early_inc_range(Info.first->uses())) { auto *CB = dyn_cast(U.getUser()); if (!CB || !CB->isCallee(&U)) return error(Info.second, "intrinsic can only be used as callee"); + // Location for error reporting. + LocTy Loc; + auto II = OverloadedIntrinsicCallLocs.find(CB); + if (II != OverloadedIntrinsicCallLocs.end()) { + Loc = II->second; + OverloadedIntrinsicCallLocs.erase(II); + } else { + Loc = Info.second; + } + SmallVector OverloadTys; if (!Intrinsic::getIntrinsicSignature(IID, CB->getFunctionType(), OverloadTys)) - return error(Info.second, "invalid intrinsic signature"); - - U.set(Intrinsic::getDeclaration(M, IID, OverloadTys)); + return error(Loc, "invalid intrinsic signature"); + Function *Intrinsic = Intrinsic::getDeclaration(M, IID, OverloadTys); + // Note: Suffix will be empty for non-overloaded intrinsics, so this + // check will always pass. For overloaded intrinsics that do not use + // a mangling suffix as well, the suffix will be empty and they will + // always pass this error check. + if (!Intrinsic->getName().ends_with(Suffix)) + return error(Loc, "invalid intrinsic name, expected @" + + Intrinsic->getName()); + U.set(Intrinsic); } Info.first->eraseFromParent(); @@ -8079,16 +8098,24 @@ bool LLParser::parseCall(Instruction *&Inst, PerFunctionState &PFS, CI->setFastMathFlags(FMF); } - if (CalleeID.Kind == ValID::t_GlobalName && - isOldDbgFormatIntrinsic(CalleeID.StrVal)) { - if (SeenNewDbgInfoFormat) { - CI->deleteValue(); - return error(CallLoc, "llvm.dbg intrinsic should not appear in a module " - "using non-intrinsic debug info"); + if (CalleeID.Kind == ValID::t_GlobalName) { + if (StringRef(CalleeID.StrVal).starts_with("llvm.")) { + // If this is a call to an intrinsic, remember its location for better + // error reporting when overloaded intrinsic resolution fails. + OverloadedIntrinsicCallLocs[CI] = CalleeID.Loc; + } + + if (isOldDbgFormatIntrinsic(CalleeID.StrVal)) { + if (SeenNewDbgInfoFormat) { + CI->deleteValue(); + return error(CallLoc, + "llvm.dbg intrinsic should not appear in a module " + "using non-intrinsic debug info"); + } + if (!SeenOldDbgInfoFormat) + M->setNewDbgInfoFormatFlag(false); + SeenOldDbgInfoFormat = true; } - if (!SeenOldDbgInfoFormat) - M->setNewDbgInfoFormatFlag(false); - SeenOldDbgInfoFormat = true; } CI->setAttributes(PAL); ForwardRefAttrGroups[CI] = FwdRefAttrGrps; diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index 82ff4e1bc7f5c..b160c5fc5be34 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -974,13 +974,14 @@ static ArrayRef findTargetSubtable(StringRef Name) { return ArrayRef(&IntrinsicNameTable[1] + TI.Offset, TI.Count); } -/// This does the actual lookup of an intrinsic ID which -/// matches the given function name. -Intrinsic::ID Function::lookupIntrinsicID(StringRef Name) { +/// This does the actual lookup of an intrinsic ID which matches the given +/// function name. +std::pair +Function::lookupIntrinsicIDAndSuffix(StringRef Name) { ArrayRef NameTable = findTargetSubtable(Name); int Idx = Intrinsic::lookupLLVMIntrinsicByName(NameTable, Name); if (Idx == -1) - return Intrinsic::not_intrinsic; + return {Intrinsic::not_intrinsic, ""}; // Intrinsic IDs correspond to the location in IntrinsicNameTable, but we have // an index into a sub-table. @@ -992,8 +993,11 @@ Intrinsic::ID Function::lookupIntrinsicID(StringRef Name) { const auto MatchSize = strlen(NameTable[Idx]); assert(Name.size() >= MatchSize && "Expected either exact or prefix match"); bool IsExactMatch = Name.size() == MatchSize; - return IsExactMatch || Intrinsic::isOverloaded(ID) ? ID - : Intrinsic::not_intrinsic; + if (IsExactMatch) + return {ID, ""}; + if (Intrinsic::isOverloaded(ID)) + return {ID, Name.drop_front(MatchSize)}; + return {Intrinsic::not_intrinsic, ""}; } void Function::updateAfterNameChange() { diff --git a/llvm/test/Assembler/intrinsic-overload-error0.ll b/llvm/test/Assembler/intrinsic-overload-error0.ll new file mode 100644 index 0000000000000..bb9d773b2bf04 --- /dev/null +++ b/llvm/test/Assembler/intrinsic-overload-error0.ll @@ -0,0 +1,12 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +; Check that intrinsic calls with mangling are error checked. +; Mix good and bad calls to demonstrate that line number tracking is required +; in the parser to report correct line number for the second (bad) call. +define void @foo(float %a, i32 %b) { + %c = call i1 @llvm.is.constant.i32(i32 0) + ; CHECK: :[[@LINE+1]]:18: error: invalid intrinsic name, expected @llvm.is.constant.f32 + %d = call i1 @llvm.is.constant.i32(float %a) + %e = call i1 @llvm.is.constant.i1(i1 false) + ret void +} diff --git a/llvm/test/Assembler/intrinsic-overload-error1.ll b/llvm/test/Assembler/intrinsic-overload-error1.ll new file mode 100644 index 0000000000000..851148a32eb4e --- /dev/null +++ b/llvm/test/Assembler/intrinsic-overload-error1.ll @@ -0,0 +1,8 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +; Check that intrinsic calls with mangling are error checked. +define void @foo(float %a, i32 %b) { + ; CHECK: :[[@LINE+1]]:18: error: invalid intrinsic name, expected @llvm.is.constant.f32 + %c = call i1 @llvm.is.constant.badsuffix(float %a) + ret void +} diff --git a/llvm/test/Assembler/intrinsic-overload.ll b/llvm/test/Assembler/intrinsic-overload.ll new file mode 100644 index 0000000000000..02fcdd1d5c55a --- /dev/null +++ b/llvm/test/Assembler/intrinsic-overload.ll @@ -0,0 +1,16 @@ +; RUN: llvm-as < %s | llvm-dis | FileCheck %s + +; Check that intrinsic calls without any mangling are converted to correct +; mangled forms. And mangled forms with correct mangling parse correctly. +define void @foo(float %a, i32 %b) { + ; CHECK: call i1 @llvm.is.constant.f32 + %c = call i1 @llvm.is.constant(float %a) + + ; CHECK: call i1 @llvm.is.constant.i32 + %d = call i1 @llvm.is.constant(i32 %b) + + ; CHECK: call i1 @llvm.is.constant.i1 + %e = call i1 @llvm.is.constant.i1(i1 false) + + ret void +}