From f4225abffdf21360c0fa29b0231818087aed699f Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Mon, 30 Sep 2024 17:55:57 +0700 Subject: [PATCH 1/4] [IR] Allow MDString in operand bundles This change implements support of metadata strings in operand bundle values. It makes possible calls like: call void @some_func(i32 %x) [ "foo"(i32 42, metadata !"abc") ] It requires some extension of the bitcode serialization. As SSA values and metadata are stored in different tables, there must be a way to distinguish them during deserialization. It is implemented by putting a special marker before the metadata index. The marker cannot be treated as a reference to any SSA value, so it unambiguously identifies metadata. It allows extending the bitcode serialization without breaking compatibility. Metadata as operand bundle values are intended to be used in floating-point function calls. They would represent the same information as now is passed by the constrained intrinsic arguments. --- llvm/docs/LangRef.rst | 6 +++--- llvm/docs/ReleaseNotes.md | 2 ++ llvm/include/llvm/AsmParser/LLParser.h | 1 + llvm/include/llvm/Bitcode/LLVMBitCodes.h | 3 +++ llvm/lib/AsmParser/LLParser.cpp | 14 +++++++++++++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 21 ++++++++++++++++++-- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 17 +++++++++++++++- llvm/test/Bitcode/operand-bundles.ll | 24 +++++++++++++++++++++++ 8 files changed, 82 insertions(+), 6 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 0c7279de06cd6..a330b80493032 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2666,8 +2666,8 @@ are grouped into a single :ref:`attribute group `. Operand Bundles --------------- -Operand bundles are tagged sets of SSA values that can be associated -with certain LLVM instructions (currently only ``call`` s and +Operand bundles are tagged sets of SSA values or metadata strings that can be +associated with certain LLVM instructions (currently only ``call`` s and ``invoke`` s). In a way they are like metadata, but dropping them is incorrect and will change program semantics. @@ -2675,7 +2675,7 @@ Syntax:: operand bundle set ::= '[' operand bundle (, operand bundle )* ']' operand bundle ::= tag '(' [ bundle operand ] (, bundle operand )* ')' - bundle operand ::= SSA value + bundle operand ::= SSA value | metadata string tag ::= string constant Operand bundles are **not** part of a function's signature, and a diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 8ac5900a7e532..dcdd7a25c7fbe 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -88,6 +88,8 @@ Changes to the LLVM IR * `llvm.nvvm.ptr.shared.to.gen` * `llvm.nvvm.ptr.constant.to.gen` * `llvm.nvvm.ptr.local.to.gen` + +* Operand bundle values can now be metadata strings. Changes to LLVM infrastructure ------------------------------ diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h index 1ef8b8ffc3966..762dc075e8b78 100644 --- a/llvm/include/llvm/AsmParser/LLParser.h +++ b/llvm/include/llvm/AsmParser/LLParser.h @@ -66,6 +66,7 @@ namespace llvm { t_EmptyArray, // No value: [] t_Constant, // Value in ConstantVal. t_ConstantSplat, // Value in ConstantVal. + t_MDString, // Value in StrVal. t_InlineAsm, // Value in FTy/StrVal/StrVal2/UIntVal. t_ConstantStruct, // Value in ConstantStructElts. t_PackedConstantStruct // Value in ConstantStructElts. diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h index cbd92fd52fc75..ba2efee941421 100644 --- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h +++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h @@ -529,6 +529,9 @@ enum PossiblyExactOperatorOptionalFlags { PEO_EXACT = 0 }; /// PossiblyDisjointInst's SubclassOptionalData contents. enum PossiblyDisjointInstOptionalFlags { PDI_DISJOINT = 0 }; +/// Mark to distinguish metadata from value in an operator bundle. +enum MetadataOperandBundleValueMarker { OB_METADATA = 0x80000000 }; + /// GetElementPtrOptionalFlags - Flags for serializing /// GEPOperator's SubclassOptionalData contents. enum GetElementPtrOptionalFlags { diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 9f2ef2e6a9311..31162829ece2f 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -3941,6 +3941,15 @@ bool LLParser::parseValID(ValID &ID, PerFunctionState *PFS, Type *ExpectedTy) { ID.Kind = ValID::t_Constant; return false; + case lltok::exclaim: { + Lex.Lex(); + ID.StrVal = Lex.getStrVal(); + if (parseToken(lltok::StringConstant, "expected string")) + return true; + ID.Kind = ValID::t_MDString; + return false; + } + case lltok::kw_asm: { // ValID ::= 'asm' SideEffect? AlignStack? IntelDialect? STRINGCONSTANT ',' // STRINGCONSTANT @@ -6212,6 +6221,11 @@ bool LLParser::convertValIDToValue(Type *Ty, ValID &ID, Value *&V, V = ConstantVector::getSplat(cast(Ty)->getElementCount(), ID.ConstantVal); return false; + case ValID::t_MDString: + if (!Ty->isMetadataTy()) + return error(ID.Loc, "invalid type for metadata string"); + V = MetadataAsValue::get(Context, MDString::get(Context, ID.StrVal)); + return false; case ValID::t_ConstantStruct: case ValID::t_PackedConstantStruct: if (StructType *ST = dyn_cast(Ty)) { diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 6f997510b0360..8ee93253bc244 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -792,6 +792,24 @@ class BitcodeReader : public BitcodeReaderBase, public GVMaterializer { return ResVal == nullptr; } + bool getValueOrMetadata(const SmallVectorImpl &Record, + unsigned &Slot, unsigned InstNum, Value *&ResVal, + BasicBlock *ConstExprInsertBB) { + if (Slot == Record.size()) + return true; + unsigned ValID = Record[Slot++]; + if (ValID != bitc::OB_METADATA) { + unsigned TypeId; + return getValueTypePair(Record, --Slot, InstNum, ResVal, TypeId, + ConstExprInsertBB); + } + if (Slot == Record.size()) + return true; + unsigned ValNo = InstNum - (unsigned)Record[Slot++]; + ResVal = MetadataAsValue::get(Context, getFnMetadataByID(ValNo)); + return false; + } + /// Read a value out of the specified record from slot 'Slot'. Increment Slot /// past the number of slots used by the value in the record. Return true if /// there is an error. @@ -6767,8 +6785,7 @@ Error BitcodeReader::parseFunctionBody(Function *F) { unsigned OpNum = 1; while (OpNum != Record.size()) { Value *Op; - unsigned OpTypeID; - if (getValueTypePair(Record, OpNum, NextValueNo, Op, OpTypeID, CurBB)) + if (getValueOrMetadata(Record, OpNum, NextValueNo, Op, CurBB)) return error("Invalid record"); Inputs.push_back(Op); } diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index d9086bfebbd2a..bec0caef58afa 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -395,6 +395,8 @@ class ModuleBitcodeWriter : public ModuleBitcodeWriterBase { void writeModuleConstants(); bool pushValueAndType(const Value *V, unsigned InstID, SmallVectorImpl &Vals); + bool pushValueOrMetadata(const Value *V, unsigned InstID, + SmallVectorImpl &Vals); void writeOperandBundles(const CallBase &CB, unsigned InstID); void pushValue(const Value *V, unsigned InstID, SmallVectorImpl &Vals); @@ -2931,6 +2933,19 @@ bool ModuleBitcodeWriter::pushValueAndType(const Value *V, unsigned InstID, return false; } +bool ModuleBitcodeWriter::pushValueOrMetadata(const Value *V, unsigned InstID, + SmallVectorImpl &Vals) { + bool IsMetadata = V->getType()->isMetadataTy(); + if (IsMetadata) { + Vals.push_back(bitc::OB_METADATA); + Metadata *MD = cast(V)->getMetadata(); + unsigned ValID = VE.getMetadataID(MD); + Vals.push_back(InstID - ValID); + return false; + } + return pushValueAndType(V, InstID, Vals); +} + void ModuleBitcodeWriter::writeOperandBundles(const CallBase &CS, unsigned InstID) { SmallVector Record; @@ -2941,7 +2956,7 @@ void ModuleBitcodeWriter::writeOperandBundles(const CallBase &CS, Record.push_back(C.getOperandBundleTagID(Bundle.getTagName())); for (auto &Input : Bundle.Inputs) - pushValueAndType(Input, InstID, Record); + pushValueOrMetadata(Input, InstID, Record); Stream.EmitRecord(bitc::FUNC_CODE_OPERAND_BUNDLE, Record); Record.clear(); diff --git a/llvm/test/Bitcode/operand-bundles.ll b/llvm/test/Bitcode/operand-bundles.ll index ab28cffd84aa2..a8e086f784c6c 100644 --- a/llvm/test/Bitcode/operand-bundles.ll +++ b/llvm/test/Bitcode/operand-bundles.ll @@ -56,6 +56,13 @@ define void @f4(i32* %ptr) { ret void } +define void @f5(i32 %x) { +entry: + call void @callee1(i32 10, i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] +; CHECK: call void @callee1(i32 10, i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] + ret void +} + ; Invoke versions of the above tests: @@ -150,3 +157,20 @@ exception: normal: ret void } + +define void @g5(ptr %ptr) personality i8 3 { +entry: + %l = load i32, ptr %ptr, align 4 + %x = add i32 42, 1 + invoke void @callee1(i32 10, i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] + to label %normal unwind label %exception +; CHECK: invoke void @callee1(i32 10, i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] + +exception: ; preds = %entry + %cleanup = landingpad i8 + cleanup + br label %normal + +normal: ; preds = %exception, %entry + ret void +} From 0e167318c422f7e4eb3efe0b59b91ff567d553f2 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Wed, 2 Oct 2024 17:27:10 +0700 Subject: [PATCH 2/4] Change parsing operand bundle values --- llvm/include/llvm/AsmParser/LLParser.h | 1 - llvm/lib/AsmParser/LLParser.cpp | 23 ++++++++--------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h index 762dc075e8b78..1ef8b8ffc3966 100644 --- a/llvm/include/llvm/AsmParser/LLParser.h +++ b/llvm/include/llvm/AsmParser/LLParser.h @@ -66,7 +66,6 @@ namespace llvm { t_EmptyArray, // No value: [] t_Constant, // Value in ConstantVal. t_ConstantSplat, // Value in ConstantVal. - t_MDString, // Value in StrVal. t_InlineAsm, // Value in FTy/StrVal/StrVal2/UIntVal. t_ConstantStruct, // Value in ConstantStructElts. t_PackedConstantStruct // Value in ConstantStructElts. diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 31162829ece2f..33d93e52fbfe2 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -3202,8 +3202,15 @@ bool LLParser::parseOptionalOperandBundles( Type *Ty = nullptr; Value *Input = nullptr; - if (parseType(Ty) || parseValue(Ty, Input, PFS)) + if (parseType(Ty)) return true; + if (Ty->isMetadataTy()) { + if (parseMetadataAsValue(Input, PFS)) + return true; + } else { + if (parseValue(Ty, Input, PFS)) + return true; + } Inputs.push_back(Input); } @@ -3941,15 +3948,6 @@ bool LLParser::parseValID(ValID &ID, PerFunctionState *PFS, Type *ExpectedTy) { ID.Kind = ValID::t_Constant; return false; - case lltok::exclaim: { - Lex.Lex(); - ID.StrVal = Lex.getStrVal(); - if (parseToken(lltok::StringConstant, "expected string")) - return true; - ID.Kind = ValID::t_MDString; - return false; - } - case lltok::kw_asm: { // ValID ::= 'asm' SideEffect? AlignStack? IntelDialect? STRINGCONSTANT ',' // STRINGCONSTANT @@ -6221,11 +6219,6 @@ bool LLParser::convertValIDToValue(Type *Ty, ValID &ID, Value *&V, V = ConstantVector::getSplat(cast(Ty)->getElementCount(), ID.ConstantVal); return false; - case ValID::t_MDString: - if (!Ty->isMetadataTy()) - return error(ID.Loc, "invalid type for metadata string"); - V = MetadataAsValue::get(Context, MDString::get(Context, ID.StrVal)); - return false; case ValID::t_ConstantStruct: case ValID::t_PackedConstantStruct: if (StructType *ST = dyn_cast(Ty)) { From 19d6fab07482e18b7e12d0f1f52bfb03420452b2 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Wed, 2 Oct 2024 20:12:54 +0700 Subject: [PATCH 3/4] Add a test to compatibility.ll --- llvm/test/Bitcode/compatibility.ll | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/llvm/test/Bitcode/compatibility.ll b/llvm/test/Bitcode/compatibility.ll index a1b2370a87b82..280c3a99d7535 100644 --- a/llvm/test/Bitcode/compatibility.ll +++ b/llvm/test/Bitcode/compatibility.ll @@ -1327,6 +1327,14 @@ continue: ret i32 0 } +declare void @instructions.bundles.callee(i32) +define void @instructions.bundles.metadata(i32 %x) { +entry: + call void @instructions.bundles.callee(i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] +; CHECK: call void @instructions.bundles.callee(i32 %x) [ "foo"(i32 42, metadata !"abc"), "bar"(metadata !"abcde", metadata !"qwerty") ] + ret void +} + ; Instructions -- Unary Operations define void @instructions.unops(double %op1) { fneg double %op1 From 7c1def3344b4900fe22360bbb639eb71cd13b6c3 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Thu, 10 Oct 2024 19:25:45 +0700 Subject: [PATCH 4/4] Reformat 'if' statement --- llvm/lib/AsmParser/LLParser.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 33d93e52fbfe2..c3b4a8235ce63 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -3207,9 +3207,8 @@ bool LLParser::parseOptionalOperandBundles( if (Ty->isMetadataTy()) { if (parseMetadataAsValue(Input, PFS)) return true; - } else { - if (parseValue(Ty, Input, PFS)) - return true; + } else if (parseValue(Ty, Input, PFS)) { + return true; } Inputs.push_back(Input); }