https://github.com/joaosaffran updated https://github.com/llvm/llvm-project/pull/135085
>From 9b59d0108f6b23c039e2c417247216862073cd4b Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Wed, 9 Apr 2025 21:05:58 +0000 Subject: [PATCH 1/9] adding support for root constants in metadata generation --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 120 +++++++++++++++++- llvm/lib/Target/DirectX/DXILRootSignature.h | 6 +- .../RootSignature-Flags-Validation-Error.ll | 7 +- .../RootSignature-RootConstants.ll | 34 +++++ ...ature-ShaderVisibility-Validation-Error.ll | 20 +++ 5 files changed, 182 insertions(+), 5 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 412ab7765a7ae..7686918b0fc75 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -40,6 +40,13 @@ static bool reportError(LLVMContext *Ctx, Twine Message, return true; } +static bool reportValueError(LLVMContext *Ctx, Twine ParamName, uint32_t Value, + DiagnosticSeverity Severity = DS_Error) { + Ctx->diagnose(DiagnosticInfoGeneric( + "Invalid value for " + ParamName + ": " + Twine(Value), Severity)); + return true; +} + static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, MDNode *RootFlagNode) { @@ -52,6 +59,45 @@ static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, return false; } +static bool extractMdValue(uint32_t &Value, MDNode *Node, unsigned int OpId) { + + auto *CI = mdconst::extract<ConstantInt>(Node->getOperand(OpId)); + if (CI == nullptr) + return true; + + Value = CI->getZExtValue(); + return false; +} + +static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, + MDNode *RootFlagNode) { + + if (RootFlagNode->getNumOperands() != 5) + return reportError(Ctx, "Invalid format for RootConstants Element"); + + mcdxbc::RootParameter NewParameter; + NewParameter.Header.ParameterType = dxbc::RootParameterType::Constants32Bit; + + uint32_t SV; + if (extractMdValue(SV, RootFlagNode, 1)) + return reportError(Ctx, "Invalid value for ShaderVisibility"); + + NewParameter.Header.ShaderVisibility = (dxbc::ShaderVisibility)SV; + + if (extractMdValue(NewParameter.Constants.ShaderRegister, RootFlagNode, 2)) + return reportError(Ctx, "Invalid value for ShaderRegister"); + + if (extractMdValue(NewParameter.Constants.RegisterSpace, RootFlagNode, 3)) + return reportError(Ctx, "Invalid value for RegisterSpace"); + + if (extractMdValue(NewParameter.Constants.Num32BitValues, RootFlagNode, 4)) + return reportError(Ctx, "Invalid value for Num32BitValues"); + + RSD.Parameters.push_back(NewParameter); + + return false; +} + static bool parseRootSignatureElement(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, MDNode *Element) { @@ -62,12 +108,16 @@ static bool parseRootSignatureElement(LLVMContext *Ctx, RootSignatureElementKind ElementKind = StringSwitch<RootSignatureElementKind>(ElementText->getString()) .Case("RootFlags", RootSignatureElementKind::RootFlags) + .Case("RootConstants", RootSignatureElementKind::RootConstants) .Default(RootSignatureElementKind::Error); switch (ElementKind) { case RootSignatureElementKind::RootFlags: return parseRootFlags(Ctx, RSD, Element); + case RootSignatureElementKind::RootConstants: + return parseRootConstants(Ctx, RSD, Element); + break; case RootSignatureElementKind::Error: return reportError(Ctx, "Invalid Root Signature Element: " + ElementText->getString()); @@ -94,10 +144,56 @@ static bool parse(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, static bool verifyRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; } +static bool verifyShaderVisibility(dxbc::ShaderVisibility Flags) { + switch (Flags) { + + case dxbc::ShaderVisibility::All: + case dxbc::ShaderVisibility::Vertex: + case dxbc::ShaderVisibility::Hull: + case dxbc::ShaderVisibility::Domain: + case dxbc::ShaderVisibility::Geometry: + case dxbc::ShaderVisibility::Pixel: + case dxbc::ShaderVisibility::Amplification: + case dxbc::ShaderVisibility::Mesh: + return true; + } + + return false; +} + +static bool verifyParameterType(dxbc::RootParameterType Flags) { + switch (Flags) { + case dxbc::RootParameterType::Constants32Bit: + return true; + } + + return false; +} + +static bool verifyVersion(uint32_t Version) { + return (Version == 1 || Version == 2); +} + static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { + + if (!verifyVersion(RSD.Header.Version)) { + return reportValueError(Ctx, "Version", RSD.Header.Version); + } + if (!verifyRootFlag(RSD.Header.Flags)) { - return reportError(Ctx, "Invalid Root Signature flag value"); + return reportValueError(Ctx, "RootFlags", RSD.Header.Flags); + } + + for (const auto &P : RSD.Parameters) { + if (!verifyShaderVisibility(P.Header.ShaderVisibility)) + return reportValueError(Ctx, "ShaderVisibility", + (uint32_t)P.Header.ShaderVisibility); + + if (!verifyParameterType(P.Header.ParameterType)) + return reportValueError(Ctx, "ParameterType", + (uint32_t)P.Header.ParameterType); } + return false; } @@ -211,6 +307,28 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n"; OS << indent(Space) << "StaticSamplersOffset: " << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n"; + + Space++; + for (auto const &P : RS.Parameters) { + OS << indent(Space) << "Parameter Type: " << &P.Header.ParameterType + << ":\n"; + OS << indent(Space) << "Shader Visibility: " << &P.Header.ShaderVisibility + << ":\n"; + OS << indent(Space) << "Parameter Offset: " << &P.Header.ParameterOffset + << ":\n"; + switch (P.Header.ParameterType) { + case dxbc::RootParameterType::Constants32Bit: + OS << indent(Space) << "Register Space: " << &P.Constants.RegisterSpace + << ":\n"; + OS << indent(Space) + << "Shader Register: " << &P.Constants.ShaderRegister << ":\n"; + OS << indent(Space) + << "Num 32 Bit Values: " << &P.Constants.Num32BitValues << ":\n"; + break; + } + } + Space--; + Space--; // end root signature header } diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.h b/llvm/lib/Target/DirectX/DXILRootSignature.h index 8c25b2eb3fadf..93ec614f1ab85 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.h +++ b/llvm/lib/Target/DirectX/DXILRootSignature.h @@ -24,7 +24,11 @@ namespace llvm { namespace dxil { -enum class RootSignatureElementKind { Error = 0, RootFlags = 1 }; +enum class RootSignatureElementKind { + Error = 0, + RootFlags = 1, + RootConstants = 2 +}; class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> { friend AnalysisInfoMixin<RootSignatureAnalysis>; static AnalysisKey Key; diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll index fe93c9993c1c3..3af9a524e77f4 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll @@ -1,6 +1,6 @@ ; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s -; CHECK: error: Invalid Root Signature flag value +; CHECK: error: Invalid value for ShaderVisibility: 255 ; CHECK-NOT: Root Signature Definitions target triple = "dxil-unknown-shadermodel6.0-compute" @@ -16,5 +16,6 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } !dx.rootsignatures = !{!2} ; list of function/root signature pairs !2 = !{ ptr @main, !3 } ; function, root signature -!3 = !{ !4 } ; list of root signature elements -!4 = !{ !"RootFlags", i32 2147487744 } ; 1 = allow_input_assembler_input_layout +!3 = !{ !4, !5 } ; list of root signature elements +!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout +!5 = !{ !"RootConstants", i32 255, i32 1, i32 2, i32 3 } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll new file mode 100644 index 0000000000000..d0520632df2a2 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll @@ -0,0 +1,34 @@ +; RUN: opt %s -dxil-embed -dxil-globals -S -o - | FileCheck %s +; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC + +target triple = "dxil-unknown-shadermodel6.0-compute" + +; CHECK: @dx.rts0 = private constant [48 x i8] c"{{.*}}", section "RTS0", align 4 + +define void @main() #0 { +entry: + ret void +} +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !4, !5 } ; list of root signature elements +!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout +!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 } + +; DXC: - Name: RTS0 +; DXC-NEXT: Size: 48 +; DXC-NEXT: RootSignature: +; DXC-NEXT: Version: 2 +; DXC-NEXT: NumStaticSamplers: 0 +; DXC-NEXT: StaticSamplersOffset: 0 +; DXC-NEXT: Parameters: +; DXC-NEXT: - ParameterType: Constants32Bit +; DXC-NEXT: ShaderVisibility: All +; DXC-NEXT: Constants: +; DXC-NEXT: Num32BitValues: 3 +; DXC-NEXT: RegisterSpace: 2 +; DXC-NEXT: ShaderRegister: 1 +; DXC-NEXT: AllowInputAssemblerInputLayout: true diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll new file mode 100644 index 0000000000000..4b8e6abacd7ad --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll @@ -0,0 +1,20 @@ +; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s + +; CHECK: error: Invalid value for RootFlags: 2147487744 +; CHECK-NOT: Root Signature Definitions + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +define void @main() #0 { +entry: + ret void +} + +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !4 } ; list of root signature elements +!4 = !{ !"RootFlags", i32 2147487744 } ; 1 = allow_input_assembler_input_layout >From efc5e52bb8843a025f49e41b322682753c061c3f Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Wed, 9 Apr 2025 21:19:25 +0000 Subject: [PATCH 2/9] add test --- llvm/lib/Target/DirectX/DXILRootSignature.cpp | 33 +++++++++---------- .../ContainerData/RootSignature-Parameters.ll | 30 +++++++++++++++++ 2 files changed, 46 insertions(+), 17 deletions(-) create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 7686918b0fc75..7f30cef239696 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -299,31 +299,30 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, // start root signature header Space++; - OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << ":\n"; - OS << indent(Space) << "Version: " << RS.Header.Version << ":\n"; - OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << ":\n"; + OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << "\n"; + OS << indent(Space) << "Version: " << RS.Header.Version << "\n"; + OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << "\n"; OS << indent(Space) << "RootParametersOffset: " << sizeof(RS.Header) - << ":\n"; - OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n"; + << "\n"; + OS << indent(Space) << "NumStaticSamplers: " << 0 << "\n"; OS << indent(Space) << "StaticSamplersOffset: " - << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n"; + << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << "\n"; Space++; for (auto const &P : RS.Parameters) { - OS << indent(Space) << "Parameter Type: " << &P.Header.ParameterType - << ":\n"; - OS << indent(Space) << "Shader Visibility: " << &P.Header.ShaderVisibility - << ":\n"; - OS << indent(Space) << "Parameter Offset: " << &P.Header.ParameterOffset - << ":\n"; + OS << indent(Space) + << "Parameter Type: " << (uint32_t)P.Header.ParameterType << "\n"; + OS << indent(Space) + << "Shader Visibility: " << (uint32_t)P.Header.ShaderVisibility + << "\n"; switch (P.Header.ParameterType) { case dxbc::RootParameterType::Constants32Bit: - OS << indent(Space) << "Register Space: " << &P.Constants.RegisterSpace - << ":\n"; + OS << indent(Space) << "Register Space: " << P.Constants.RegisterSpace + << "\n"; + OS << indent(Space) << "Shader Register: " << P.Constants.ShaderRegister + << "\n"; OS << indent(Space) - << "Shader Register: " << &P.Constants.ShaderRegister << ":\n"; - OS << indent(Space) - << "Num 32 Bit Values: " << &P.Constants.Num32BitValues << ":\n"; + << "Num 32 Bit Values: " << P.Constants.Num32BitValues << "\n"; break; } } diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll new file mode 100644 index 0000000000000..9a2f7d840a236 --- /dev/null +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll @@ -0,0 +1,30 @@ +; RUN: opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s + +target triple = "dxil-unknown-shadermodel6.0-compute" + + +define void @main() #0 { +entry: + ret void +} + +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } + +!dx.rootsignatures = !{!2} ; list of function/root signature pairs +!2 = !{ ptr @main, !3 } ; function, root signature +!3 = !{ !4, !5 } ; list of root signature elements +!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout +!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 } + +; CHECK-LABEL: Definition for 'main': +; CHECK-NEXT: Flags: 0x000001 +; CHECK-NEXT: Version: 2 +; CHECK-NEXT: NumParameters: 1 +; CHECK-NEXT: RootParametersOffset: 24 +; CHECK-NEXT: NumStaticSamplers: 0 +; CHECK-NEXT: StaticSamplersOffset: 48 +; CHECK-NEXT: Parameter Type: 1 +; CHECK-NEXT: Shader Visibility: 0 +; CHECK-NEXT: Register Space: 2 +; CHECK-NEXT: Shader Register: 1 +; CHECK-NEXT: Num 32 Bit Values: 3 >From e6865a76f73e172304f64d1bdd7225f7ff33a449 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Wed, 9 Apr 2025 23:37:34 +0000 Subject: [PATCH 3/9] Revert "clean up" This reverts commit cfc6bfb3fcb5a1739b4c304a273d8ce4cd651c56. --- llvm/include/llvm/BinaryFormat/DXContainer.h | 2 +- llvm/include/llvm/MC/DXContainerRootSignature.h | 1 + llvm/include/llvm/Object/DXContainer.h | 3 +++ llvm/lib/MC/DXContainerRootSignature.cpp | 1 + llvm/lib/Object/DXContainer.cpp | 2 ++ llvm/lib/ObjectYAML/DXContainerYAML.cpp | 3 +++ llvm/unittests/Object/DXContainerTest.cpp | 3 +++ 7 files changed, 14 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index e0a03d6d777a8..f9140b6a8c805 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -55,7 +55,7 @@ enum class HashFlags : uint32_t { }; struct ShaderHash { - uint32_t Flags; // dxbc::HashFlags + uint32_t Flags; // HashFlags uint8_t Digest[16]; bool isPopulated(); diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 0ad7b21981415..6cf210728c7e8 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Support/ErrorHandling.h" #include <cstdint> #include <limits> diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index 5a20679f05c19..cf7a9d5be93ae 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -23,6 +23,9 @@ #include "llvm/Support/MemoryBufferRef.h" #include "llvm/TargetParser/Triple.h" #include <array> +#include <cstddef> +#include <cstdint> +#include <memory> #include <variant> namespace llvm { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index a846e4572c449..e5f67df21e8ef 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -8,6 +8,7 @@ #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/ADT/SmallString.h" +#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Support/EndianStream.h" using namespace llvm; diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp index 2e014dd0c0c53..04d124b6bb302 100644 --- a/llvm/lib/Object/DXContainer.cpp +++ b/llvm/lib/Object/DXContainer.cpp @@ -11,7 +11,9 @@ #include "llvm/Object/Error.h" #include "llvm/Support/Alignment.h" #include "llvm/Support/Endian.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" +#include <cstdint> using namespace llvm; using namespace llvm::object; diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index e8578df8aa509..3380c80d130de 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -14,6 +14,9 @@ #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Object/DXContainer.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" namespace llvm { diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index b2ff9ad218de0..03d3d6d73bbdc 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -8,9 +8,12 @@ #include "llvm/Object/DXContainer.h" #include "llvm/ADT/StringRef.h" +#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/BinaryFormat/Magic.h" #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ObjectYAML/yaml2obj.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" >From 021976d8a2e3836c940a7b7b3acd0f5ed0c044c1 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Wed, 9 Apr 2025 23:39:48 +0000 Subject: [PATCH 4/9] Reapply "clean up" This reverts commit e6865a76f73e172304f64d1bdd7225f7ff33a449. --- llvm/include/llvm/BinaryFormat/DXContainer.h | 2 +- llvm/include/llvm/MC/DXContainerRootSignature.h | 1 - llvm/include/llvm/Object/DXContainer.h | 3 --- llvm/lib/MC/DXContainerRootSignature.cpp | 1 - llvm/lib/Object/DXContainer.cpp | 2 -- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 3 --- llvm/unittests/Object/DXContainerTest.cpp | 3 --- 7 files changed, 1 insertion(+), 14 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index f9140b6a8c805..e0a03d6d777a8 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -55,7 +55,7 @@ enum class HashFlags : uint32_t { }; struct ShaderHash { - uint32_t Flags; // HashFlags + uint32_t Flags; // dxbc::HashFlags uint8_t Digest[16]; bool isPopulated(); diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 6cf210728c7e8..0ad7b21981415 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "llvm/BinaryFormat/DXContainer.h" -#include "llvm/Support/ErrorHandling.h" #include <cstdint> #include <limits> diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index cf7a9d5be93ae..5a20679f05c19 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -23,9 +23,6 @@ #include "llvm/Support/MemoryBufferRef.h" #include "llvm/TargetParser/Triple.h" #include <array> -#include <cstddef> -#include <cstdint> -#include <memory> #include <variant> namespace llvm { diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index e5f67df21e8ef..a846e4572c449 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -8,7 +8,6 @@ #include "llvm/MC/DXContainerRootSignature.h" #include "llvm/ADT/SmallString.h" -#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Support/EndianStream.h" using namespace llvm; diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp index 04d124b6bb302..2e014dd0c0c53 100644 --- a/llvm/lib/Object/DXContainer.cpp +++ b/llvm/lib/Object/DXContainer.cpp @@ -11,9 +11,7 @@ #include "llvm/Object/Error.h" #include "llvm/Support/Alignment.h" #include "llvm/Support/Endian.h" -#include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" -#include <cstdint> using namespace llvm; using namespace llvm::object; diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 3380c80d130de..e8578df8aa509 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -14,9 +14,6 @@ #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" -#include "llvm/Object/DXContainer.h" -#include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" namespace llvm { diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index 03d3d6d73bbdc..b2ff9ad218de0 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -8,12 +8,9 @@ #include "llvm/Object/DXContainer.h" #include "llvm/ADT/StringRef.h" -#include "llvm/BinaryFormat/DXContainer.h" #include "llvm/BinaryFormat/Magic.h" #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ObjectYAML/yaml2obj.h" -#include "llvm/Support/Casting.h" -#include "llvm/Support/Error.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" >From 102ff4bea5846b6737884e6f7114f5e223e9fa35 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Thu, 10 Apr 2025 00:09:08 +0000 Subject: [PATCH 5/9] removing unnecessary parameters --- llvm/include/llvm/Object/DXContainer.h | 18 ++++++++---------- llvm/lib/Object/DXContainer.cpp | 10 +--------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index 5a20679f05c19..eeee443cb9460 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -170,13 +170,12 @@ class RootSignature { uint32_t StaticSamplersOffset; uint32_t Flags; ViewArray<dxbc::RootParameterHeader> ParametersHeaders; - uint32_t ParameterSpaceOffset; - StringRef ParameterSpace; + StringRef PartData; using param_header_iterator = ViewArray<dxbc::RootParameterHeader>::iterator; public: - RootSignature() {} + RootSignature(StringRef PD) : PartData(PD) {} Error parse(StringRef Data); uint32_t getVersion() const { return Version; } @@ -191,10 +190,6 @@ class RootSignature { llvm::Expected<RootParameterView> getParameter(const dxbc::RootParameterHeader &Header) const { - assert(ParameterSpaceOffset != 0 && - "This should be initialized before reading parameters"); - size_t CorrectOffset = Header.ParameterOffset - ParameterSpaceOffset; - StringRef Data; size_t DataSize; switch (Header.ParameterType) { @@ -202,12 +197,15 @@ class RootSignature { DataSize = sizeof(dxbc::RootConstants); break; } + auto EndOfSectionByte = getNumStaticSamplers() == 0 + ? PartData.size() + : getStaticSamplersOffset(); - if (CorrectOffset + DataSize > ParameterSpace.size()) + if (Header.ParameterOffset + DataSize > EndOfSectionByte) return parseFailed("Reading structure out of file bounds"); - Data = ParameterSpace.substr(CorrectOffset, DataSize); - RootParameterView View = RootParameterView(Header, Data); + StringRef Buff = PartData.substr(Header.ParameterOffset, DataSize); + RootParameterView View = RootParameterView(Header, Buff); return View; } }; diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp index 2e014dd0c0c53..88e548267f97b 100644 --- a/llvm/lib/Object/DXContainer.cpp +++ b/llvm/lib/Object/DXContainer.cpp @@ -96,7 +96,7 @@ Error DXContainer::parseHash(StringRef Part) { Error DXContainer::parseRootSignature(StringRef Part) { if (RootSignature) return parseFailed("More than one RTS0 part is present in the file"); - RootSignature = DirectX::RootSignature(); + RootSignature = DirectX::RootSignature(Part); if (Error Err = RootSignature->parse(Part)) return Err; return Error::success(); @@ -278,14 +278,6 @@ Error DirectX::RootSignature::parse(StringRef Data) { ParametersHeaders.Data = Data.substr( RootParametersOffset, NumParameters * sizeof(dxbc::RootParameterHeader)); - ParameterSpaceOffset = - RootParametersOffset + NumParameters * sizeof(dxbc::RootParameterHeader); - size_t ParameterSpaceEnd = - (NumStaticSamplers == 0) ? Data.size() : StaticSamplersOffset; - - ParameterSpace = Data.substr(ParameterSpaceOffset, - ParameterSpaceEnd - ParameterSpaceOffset); - return Error::success(); } >From 39d4b08570c55201cd6b57a6a520b01f7d04cd48 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Thu, 10 Apr 2025 21:23:22 +0000 Subject: [PATCH 6/9] addressing pr comments --- llvm/include/llvm/BinaryFormat/DXContainer.h | 33 ++++++++++-- .../llvm/MC/DXContainerRootSignature.h | 6 +-- llvm/include/llvm/Object/DXContainer.h | 12 +---- .../include/llvm/ObjectYAML/DXContainerYAML.h | 4 ++ llvm/lib/MC/DXContainerRootSignature.cpp | 4 +- llvm/lib/ObjectYAML/DXContainerEmitter.cpp | 4 +- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 53 +++++++++++++------ llvm/lib/Target/DirectX/DXILRootSignature.cpp | 18 ++++--- llvm/unittests/Object/DXContainerTest.cpp | 2 +- 9 files changed, 88 insertions(+), 48 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index e0a03d6d777a8..e23234603667b 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -14,6 +14,7 @@ #define LLVM_BINARYFORMAT_DXCONTAINER_H #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" #include "llvm/Support/SwapByteOrder.h" #include "llvm/TargetParser/Triple.h" @@ -158,19 +159,43 @@ enum class RootElementFlag : uint32_t { }; #define ROOT_PARAMETER(Val, Enum) Enum = Val, -enum class RootParameterType : uint32_t { +enum RootParameterType : uint32_t { #include "DXContainerConstants.def" }; ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes(); +#define ROOT_PARAMETER(Val, Enum) \ + case Val: \ + return dxbc::RootParameterType::Enum; +inline llvm::Expected<dxbc::RootParameterType> +safeParseParameterType(uint32_t V) { + switch (V) { +#include "DXContainerConstants.def" + } + return createStringError(std::errc::invalid_argument, + "Invalid value for parameter type"); +} + #define SHADER_VISIBILITY(Val, Enum) Enum = Val, -enum class ShaderVisibility : uint32_t { +enum ShaderVisibility : uint32_t { #include "DXContainerConstants.def" }; ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility(); +#define SHADER_VISIBILITY(Val, Enum) \ + case Val: \ + return dxbc::ShaderVisibility::Enum; +inline llvm::Expected<dxbc::ShaderVisibility> +safeParseShaderVisibility(uint32_t V) { + switch (V) { +#include "DXContainerConstants.def" + } + return createStringError(std::errc::invalid_argument, + "Invalid value for parameter type"); +} + PartType parsePartType(StringRef S); struct VertexPSVInfo { @@ -575,8 +600,8 @@ struct RootConstants { }; struct RootParameterHeader { - RootParameterType ParameterType; - ShaderVisibility ShaderVisibility; + uint32_t ParameterType; + uint32_t ShaderVisibility; uint32_t ParameterOffset; void swapBytes() { diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h index 0ad7b21981415..d0f5e2af861d8 100644 --- a/llvm/include/llvm/MC/DXContainerRootSignature.h +++ b/llvm/include/llvm/MC/DXContainerRootSignature.h @@ -23,11 +23,9 @@ struct RootParameter { }; struct RootSignatureDesc { - dxbc::RootSignatureHeader Header; + uint32_t Version = 2U; + uint32_t Flags = 0U; SmallVector<mcdxbc::RootParameter> Parameters; - RootSignatureDesc() - : Header(dxbc::RootSignatureHeader{ - 2, 0, sizeof(dxbc::RootSignatureHeader), 0, 0, 0}) {} void write(raw_ostream &OS) const; diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index eeee443cb9460..656ef864c9473 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -117,16 +117,6 @@ template <typename T> struct ViewArray { }; namespace DirectX { - -struct RootParameter { - dxbc::RootParameterHeader Header; - union { - dxbc::RootConstants Constants; - }; - - RootParameter() = default; -}; - struct RootParameterView { const dxbc::RootParameterHeader &Header; StringRef ParamData; @@ -183,7 +173,7 @@ class RootSignature { uint32_t getRootParametersOffset() const { return RootParametersOffset; } uint32_t getNumStaticSamplers() const { return NumStaticSamplers; } uint32_t getStaticSamplersOffset() const { return StaticSamplersOffset; } - llvm::iterator_range<param_header_iterator> param_header() const { + llvm::iterator_range<param_header_iterator> param_headers() const { return llvm::make_range(ParametersHeaders.begin(), ParametersHeaders.end()); } uint32_t getFlags() const { return Flags; } diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index 9d453d89bf73d..baafbdc6c9151 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -103,6 +103,10 @@ struct RootSignatureYamlDesc { uint32_t getEncodedFlags(); + iterator_range<RootParameterYamlDesc *> params() { + return make_range(Parameters.begin(), Parameters.end()); + } + #include "llvm/BinaryFormat/DXContainerConstants.def" }; diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp index a846e4572c449..63de60acc9bc2 100644 --- a/llvm/lib/MC/DXContainerRootSignature.cpp +++ b/llvm/lib/MC/DXContainerRootSignature.cpp @@ -51,13 +51,13 @@ void RootSignatureDesc::write(raw_ostream &OS) const { const uint32_t StaticSamplerOffset = 0u; const uint32_t NumStaticSamplers = 0u; - support::endian::write(BOS, Header.Version, llvm::endianness::little); + support::endian::write(BOS, Version, llvm::endianness::little); support::endian::write(BOS, NumParameters, llvm::endianness::little); support::endian::write(BOS, (uint32_t)sizeof(dxbc::RootSignatureHeader), llvm::endianness::little); support::endian::write(BOS, StaticSamplerOffset, llvm::endianness::little); support::endian::write(BOS, NumStaticSamplers, llvm::endianness::little); - support::endian::write(BOS, Header.Flags, llvm::endianness::little); + support::endian::write(BOS, Flags, llvm::endianness::little); SmallVector<uint32_t> ParamsOffsets; for (const auto &P : Parameters) { diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp index 2092dd9f1e6e5..1342629df84f3 100644 --- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp +++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp @@ -267,8 +267,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) { continue; mcdxbc::RootSignatureDesc RS; - RS.Header.Flags = P.RootSignature->getEncodedFlags(); - RS.Header.Version = P.RootSignature->Version; + RS.Flags = P.RootSignature->getEncodedFlags(); + RS.Version = P.RootSignature->Version; for (const auto &Param : P.RootSignature->Parameters) { mcdxbc::RootParameter NewParam; NewParam.Header = dxbc::RootParameterHeader{ diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index e8578df8aa509..0791cccd244b2 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -35,27 +35,48 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc( NumStaticSamplers(Data.getNumStaticSamplers()), StaticSamplersOffset(Data.getStaticSamplersOffset()) { uint32_t Flags = Data.getFlags(); - for (const auto &PH : Data.param_header()) { + for (const auto &PH : Data.param_headers()) { RootParameterYamlDesc NewP; NewP.Offset = PH.ParameterOffset; - NewP.Type = PH.ParameterType; - NewP.Visibility = PH.ShaderVisibility; - llvm::Expected<object::DirectX::RootParameterView> ParamView = + llvm::Expected<dxbc::RootParameterType> TypeOrErr = + dxbc::safeParseParameterType(PH.ParameterType); + if (Error E = TypeOrErr.takeError()) { + llvm::errs() << "Error: " << E << "\n"; + continue; + } + + NewP.Type = TypeOrErr.get(); + + llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr = + dxbc::safeParseShaderVisibility(PH.ShaderVisibility); + if (Error E = VisibilityOrErr.takeError()) { + llvm::errs() << "Error: " << E << "\n"; + continue; + } + NewP.Visibility = VisibilityOrErr.get(); + + llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr = Data.getParameter(PH); - if (!ParamView) - llvm::errs() << "Error: " << ParamView.takeError() << "\n"; - auto PV = *ParamView; - - if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&PV)) { - auto Constants = RCV->read(); - if (!Constants) - llvm::errs() << "Error: " << Constants.takeError() << "\n"; - - NewP.Constants.Num32BitValues = Constants->Num32BitValues; - NewP.Constants.ShaderRegister = Constants->ShaderRegister; - NewP.Constants.RegisterSpace = Constants->RegisterSpace; + if (Error E = ParamViewOrErr.takeError()) { + llvm::errs() << "Error: " << E << "\n"; + continue; + } + object::DirectX::RootParameterView ParamView = ParamViewOrErr.get(); + + if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&ParamView)) { + llvm::Expected<dxbc::RootConstants> ConstantsOrErr = RCV->read(); + if (Error E = ConstantsOrErr.takeError()) { + llvm::errs() << "Error: " << E << "\n"; + continue; + } + + auto Constants = *ConstantsOrErr; + + NewP.Constants.Num32BitValues = Constants.Num32BitValues; + NewP.Constants.ShaderRegister = Constants.ShaderRegister; + NewP.Constants.RegisterSpace = Constants.RegisterSpace; } Parameters.push_back(NewP); } diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp index 412ab7765a7ae..3ba0535e0114b 100644 --- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp +++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp @@ -47,7 +47,7 @@ static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, return reportError(Ctx, "Invalid format for RootFlag Element"); auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1)); - RSD.Header.Flags = Flag->getZExtValue(); + RSD.Flags = Flag->getZExtValue(); return false; } @@ -95,7 +95,7 @@ static bool parse(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD, static bool verifyRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; } static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) { - if (!verifyRootFlag(RSD.Header.Flags)) { + if (!verifyRootFlag(RSD.Flags)) { return reportError(Ctx, "Invalid Root Signature flag value"); } return false; @@ -191,6 +191,8 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> &RSDMap = AM.getResult<RootSignatureAnalysis>(M); + + const size_t RSHSize = sizeof(dxbc::RootSignatureHeader); OS << "Root Signature Definitions" << "\n"; uint8_t Space = 0; @@ -203,14 +205,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M, // start root signature header Space++; - OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << ":\n"; - OS << indent(Space) << "Version: " << RS.Header.Version << ":\n"; + OS << indent(Space) << "Flags: " << format_hex(RS.Flags, 8) << ":\n"; + OS << indent(Space) << "Version: " << RS.Version << ":\n"; OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << ":\n"; - OS << indent(Space) << "RootParametersOffset: " << sizeof(RS.Header) - << ":\n"; + OS << indent(Space) << "RootParametersOffset: " << RSHSize << ":\n"; OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n"; - OS << indent(Space) << "StaticSamplersOffset: " - << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n"; + OS << indent(Space) + << "StaticSamplersOffset: " << RSHSize + RS.Parameters.size_in_bytes() + << ":\n"; Space--; // end root signature header } diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index b2ff9ad218de0..bf32c07951520 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -890,7 +890,7 @@ TEST(RootSignature, ParseRootConstant) { ASSERT_EQ(RS.getStaticSamplersOffset(), 44u); ASSERT_EQ(RS.getFlags(), 17u); - auto RootParam = *RS.param_header().begin(); + auto RootParam = *RS.param_headers().begin(); ASSERT_EQ((unsigned)RootParam.ParameterType, 1u); ASSERT_EQ((unsigned)RootParam.ShaderVisibility, 2u); auto ParamView = RS.getParameter(RootParam); >From 7343d953a60e9f07fdb1c54c988690cfbff51bae Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Fri, 11 Apr 2025 19:18:29 +0000 Subject: [PATCH 7/9] adding more tests --- .../include/llvm/ObjectYAML/DXContainerYAML.h | 8 +++-- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 34 +++++++++++-------- llvm/tools/obj2yaml/dxcontainer2yaml.cpp | 9 +++-- llvm/unittests/Object/DXContainerTest.cpp | 23 +++++++++++++ .../ObjectYAML/DXContainerYAMLTest.cpp | 1 + 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index baafbdc6c9151..4718b5fd61adf 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -92,9 +92,6 @@ struct RootParameterYamlDesc { }; struct RootSignatureYamlDesc { - RootSignatureYamlDesc() = default; - RootSignatureYamlDesc(const object::DirectX::RootSignature &Data); - uint32_t Version; uint32_t NumStaticSamplers; uint32_t StaticSamplersOffset; @@ -107,7 +104,12 @@ struct RootSignatureYamlDesc { return make_range(Parameters.begin(), Parameters.end()); } + static llvm::Expected<DXContainerYAML::RootSignatureYamlDesc> + create(const object::DirectX::RootSignature &Data); + #include "llvm/BinaryFormat/DXContainerConstants.def" + + RootSignatureYamlDesc() = default; }; using ResourceFlags = dxbc::PSV::ResourceFlags; diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 0791cccd244b2..e80e4c3543da4 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -14,7 +14,9 @@ #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/BinaryFormat/DXContainer.h" +#include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" +#include <utility> namespace llvm { @@ -29,11 +31,16 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) { #include "llvm/BinaryFormat/DXContainerConstants.def" } -DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc( - const object::DirectX::RootSignature &Data) - : Version(Data.getVersion()), - NumStaticSamplers(Data.getNumStaticSamplers()), - StaticSamplersOffset(Data.getStaticSamplersOffset()) { +llvm::Expected<DXContainerYAML::RootSignatureYamlDesc> +DXContainerYAML::RootSignatureYamlDesc::create( + const object::DirectX::RootSignature &Data) { + + RootSignatureYamlDesc RootSigDesc; + + RootSigDesc.Version = Data.getVersion(); + RootSigDesc.NumStaticSamplers = Data.getNumStaticSamplers(); + RootSigDesc.StaticSamplersOffset = Data.getStaticSamplersOffset(); + uint32_t Flags = Data.getFlags(); for (const auto &PH : Data.param_headers()) { @@ -43,8 +50,7 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc( llvm::Expected<dxbc::RootParameterType> TypeOrErr = dxbc::safeParseParameterType(PH.ParameterType); if (Error E = TypeOrErr.takeError()) { - llvm::errs() << "Error: " << E << "\n"; - continue; + return std::move(E); } NewP.Type = TypeOrErr.get(); @@ -52,24 +58,21 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc( llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr = dxbc::safeParseShaderVisibility(PH.ShaderVisibility); if (Error E = VisibilityOrErr.takeError()) { - llvm::errs() << "Error: " << E << "\n"; - continue; + return std::move(E); } NewP.Visibility = VisibilityOrErr.get(); llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr = Data.getParameter(PH); if (Error E = ParamViewOrErr.takeError()) { - llvm::errs() << "Error: " << E << "\n"; - continue; + return std::move(E); } object::DirectX::RootParameterView ParamView = ParamViewOrErr.get(); if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&ParamView)) { llvm::Expected<dxbc::RootConstants> ConstantsOrErr = RCV->read(); if (Error E = ConstantsOrErr.takeError()) { - llvm::errs() << "Error: " << E << "\n"; - continue; + return std::move(E); } auto Constants = *ConstantsOrErr; @@ -78,11 +81,12 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc( NewP.Constants.ShaderRegister = Constants.ShaderRegister; NewP.Constants.RegisterSpace = Constants.RegisterSpace; } - Parameters.push_back(NewP); + RootSigDesc.Parameters.push_back(NewP); } #define ROOT_ELEMENT_FLAG(Num, Val) \ - Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0; + RootSigDesc.Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0; #include "llvm/BinaryFormat/DXContainerConstants.def" + return RootSigDesc; } uint32_t DXContainerYAML::RootSignatureYamlDesc::getEncodedFlags() { diff --git a/llvm/tools/obj2yaml/dxcontainer2yaml.cpp b/llvm/tools/obj2yaml/dxcontainer2yaml.cpp index f3ef1b6a27bcf..c727595406767 100644 --- a/llvm/tools/obj2yaml/dxcontainer2yaml.cpp +++ b/llvm/tools/obj2yaml/dxcontainer2yaml.cpp @@ -155,8 +155,13 @@ dumpDXContainer(MemoryBufferRef Source) { break; case dxbc::PartType::RTS0: std::optional<DirectX::RootSignature> RS = Container.getRootSignature(); - if (RS.has_value()) - NewPart.RootSignature = DXContainerYAML::RootSignatureYamlDesc(*RS); + if (RS.has_value()) { + auto RootSigDescOrErr = + DXContainerYAML::RootSignatureYamlDesc::create(*RS); + if (Error E = RootSigDescOrErr.takeError()) + return std::move(E); + NewPart.RootSignature = RootSigDescOrErr.get(); + } break; } } diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index bf32c07951520..59591435480bc 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -7,12 +7,15 @@ //===----------------------------------------------------------------------===// #include "llvm/Object/DXContainer.h" +#include "../../tools/obj2yaml/dxcontainer2yaml.cpp" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/BinaryFormat/Magic.h" #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ObjectYAML/yaml2obj.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Testing/Support/Error.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" using namespace llvm; @@ -907,4 +910,24 @@ TEST(RootSignature, ParseRootConstant) { ASSERT_EQ(Constants->RegisterSpace, 14u); ASSERT_EQ(Constants->Num32BitValues, 16u); } + { + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00, + 0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + + SmallString<256> Storage; + raw_svector_ostream OS(Storage); + EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)), + FailedWithMessage("Invalid value for parameter type")); + } } diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp index 3e5b79be2ccff..73b2555dfd9a9 100644 --- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp +++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/ObjectYAML/ObjectYAML.h" >From 69869451be4e9e9ab4f6bdd9468ab02728670f29 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Fri, 11 Apr 2025 19:43:26 +0000 Subject: [PATCH 8/9] adding more tests and addressing comments --- llvm/include/llvm/BinaryFormat/DXContainer.h | 16 +++++-------- .../include/llvm/ObjectYAML/DXContainerYAML.h | 4 ++-- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 23 ++++++++----------- llvm/unittests/Object/DXContainerTest.cpp | 22 ++++++++++++++++-- .../ObjectYAML/DXContainerYAMLTest.cpp | 1 - 5 files changed, 38 insertions(+), 28 deletions(-) diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h index e23234603667b..56df554b56e2e 100644 --- a/llvm/include/llvm/BinaryFormat/DXContainer.h +++ b/llvm/include/llvm/BinaryFormat/DXContainer.h @@ -167,14 +167,12 @@ ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes(); #define ROOT_PARAMETER(Val, Enum) \ case Val: \ - return dxbc::RootParameterType::Enum; -inline llvm::Expected<dxbc::RootParameterType> -safeParseParameterType(uint32_t V) { + return true; +inline bool isValidParameterType(uint32_t V) { switch (V) { #include "DXContainerConstants.def" } - return createStringError(std::errc::invalid_argument, - "Invalid value for parameter type"); + return false; } #define SHADER_VISIBILITY(Val, Enum) Enum = Val, @@ -186,14 +184,12 @@ ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility(); #define SHADER_VISIBILITY(Val, Enum) \ case Val: \ - return dxbc::ShaderVisibility::Enum; -inline llvm::Expected<dxbc::ShaderVisibility> -safeParseShaderVisibility(uint32_t V) { + return true; +inline bool isValidShaderVisibility(uint32_t V) { switch (V) { #include "DXContainerConstants.def" } - return createStringError(std::errc::invalid_argument, - "Invalid value for parameter type"); + return false; } PartType parsePartType(StringRef S); diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index 4718b5fd61adf..c57a879865011 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -92,6 +92,8 @@ struct RootParameterYamlDesc { }; struct RootSignatureYamlDesc { + RootSignatureYamlDesc() = default; + uint32_t Version; uint32_t NumStaticSamplers; uint32_t StaticSamplersOffset; @@ -108,8 +110,6 @@ struct RootSignatureYamlDesc { create(const object::DirectX::RootSignature &Data); #include "llvm/BinaryFormat/DXContainerConstants.def" - - RootSignatureYamlDesc() = default; }; using ResourceFlags = dxbc::PSV::ResourceFlags; diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index e80e4c3543da4..7d43e6c7f66b8 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -16,7 +16,7 @@ #include "llvm/BinaryFormat/DXContainer.h" #include "llvm/Support/Error.h" #include "llvm/Support/ScopedPrinter.h" -#include <utility> +#include <system_error> namespace llvm { @@ -47,20 +47,17 @@ DXContainerYAML::RootSignatureYamlDesc::create( RootParameterYamlDesc NewP; NewP.Offset = PH.ParameterOffset; - llvm::Expected<dxbc::RootParameterType> TypeOrErr = - dxbc::safeParseParameterType(PH.ParameterType); - if (Error E = TypeOrErr.takeError()) { - return std::move(E); - } + if (!dxbc::isValidParameterType(PH.ParameterType)) + return createStringError(std::errc::invalid_argument, + "Invalid value for parameter type"); - NewP.Type = TypeOrErr.get(); + NewP.Type = (dxbc::RootParameterType)PH.ParameterType; - llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr = - dxbc::safeParseShaderVisibility(PH.ShaderVisibility); - if (Error E = VisibilityOrErr.takeError()) { - return std::move(E); - } - NewP.Visibility = VisibilityOrErr.get(); + if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility)) + return createStringError(std::errc::invalid_argument, + "Invalid value for shader visibility"); + + NewP.Visibility = (dxbc::ShaderVisibility)PH.ShaderVisibility; llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr = Data.getParameter(PH); diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index 59591435480bc..e8c299f4d5c08 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -8,14 +8,12 @@ #include "llvm/Object/DXContainer.h" #include "../../tools/obj2yaml/dxcontainer2yaml.cpp" -#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/BinaryFormat/Magic.h" #include "llvm/ObjectYAML/DXContainerYAML.h" #include "llvm/ObjectYAML/yaml2obj.h" #include "llvm/Support/MemoryBufferRef.h" #include "llvm/Testing/Support/Error.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" using namespace llvm; @@ -930,4 +928,24 @@ TEST(RootSignature, ParseRootConstant) { EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)), FailedWithMessage("Invalid value for parameter type")); } + { + uint8_t Buffer[] = { + 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, + 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, + 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, + 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00, + 0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00}; + + SmallString<256> Storage; + raw_svector_ostream OS(Storage); + EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)), + FailedWithMessage("Invalid value for shader visibility")); + } } diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp index 73b2555dfd9a9..3e5b79be2ccff 100644 --- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp +++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp @@ -6,7 +6,6 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/ObjectYAML/ObjectYAML.h" >From c0ac522ce0000f58092c99b66bc94150aed70122 Mon Sep 17 00:00:00 2001 From: joaosaffran <joao.saff...@microsoft.com> Date: Mon, 14 Apr 2025 18:39:35 +0000 Subject: [PATCH 9/9] adding tests and changing parameter type --- llvm/include/llvm/Object/DXContainer.h | 3 +- .../include/llvm/ObjectYAML/DXContainerYAML.h | 8 ++-- llvm/lib/Object/DXContainer.cpp | 13 +++--- llvm/lib/ObjectYAML/DXContainerYAML.cpp | 20 +++------ .../ContainerData/RootSignature-Flags.ll | 2 + .../DXContainer/RootSignature-Flags.yaml | 4 ++ .../RootSignature-InvalidType.yaml | 29 +++++++++++++ .../RootSignature-InvalidVisibility.yaml | 33 +++++++++++++++ .../RootSignature-MultipleParameters.yaml | 20 +++++---- llvm/unittests/Object/DXContainerTest.cpp | 41 ------------------- .../ObjectYAML/DXContainerYAMLTest.cpp | 8 +++- 11 files changed, 104 insertions(+), 77 deletions(-) create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h index 656ef864c9473..561e4c1d1c634 100644 --- a/llvm/include/llvm/Object/DXContainer.h +++ b/llvm/include/llvm/Object/DXContainer.h @@ -167,12 +167,13 @@ class RootSignature { public: RootSignature(StringRef PD) : PartData(PD) {} - Error parse(StringRef Data); + Error parse(); uint32_t getVersion() const { return Version; } uint32_t getNumParameters() const { return NumParameters; } uint32_t getRootParametersOffset() const { return RootParametersOffset; } uint32_t getNumStaticSamplers() const { return NumStaticSamplers; } uint32_t getStaticSamplersOffset() const { return StaticSamplersOffset; } + uint32_t getNumRootParameters() const { return ParametersHeaders.size(); } llvm::iterator_range<param_header_iterator> param_headers() const { return llvm::make_range(ParametersHeaders.begin(), ParametersHeaders.end()); } diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h index c57a879865011..393bba9c79bf8 100644 --- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h +++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h @@ -82,8 +82,8 @@ struct RootConstantsYaml { }; struct RootParameterYamlDesc { - dxbc::RootParameterType Type; - dxbc::ShaderVisibility Visibility; + uint32_t Type; + uint32_t Visibility; uint32_t Offset; union { @@ -95,6 +95,8 @@ struct RootSignatureYamlDesc { RootSignatureYamlDesc() = default; uint32_t Version; + uint32_t NumRootParameters; + uint32_t RootParametersOffset; uint32_t NumStaticSamplers; uint32_t StaticSamplersOffset; @@ -224,8 +226,6 @@ LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ResourceKind) LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::D3DSystemValue) LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigComponentType) LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigMinPrecision) -LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::RootParameterType) -LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::ShaderVisibility) namespace llvm { diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp index 88e548267f97b..77303edce1a41 100644 --- a/llvm/lib/Object/DXContainer.cpp +++ b/llvm/lib/Object/DXContainer.cpp @@ -97,7 +97,7 @@ Error DXContainer::parseRootSignature(StringRef Part) { if (RootSignature) return parseFailed("More than one RTS0 part is present in the file"); RootSignature = DirectX::RootSignature(Part); - if (Error Err = RootSignature->parse(Part)) + if (Error Err = RootSignature->parse()) return Err; return Error::success(); } @@ -242,12 +242,11 @@ void DXContainer::PartIterator::updateIteratorImpl(const uint32_t Offset) { IteratorState.Offset = Offset; } -Error DirectX::RootSignature::parse(StringRef Data) { - const char *Begin = Data.begin(); - const char *Current = Data.begin(); +Error DirectX::RootSignature::parse() { + const char *Current = PartData.begin(); // Root Signature headers expects 6 integers to be present. - if (Data.size() < 6 * sizeof(uint32_t)) + if (PartData.size() < 6 * sizeof(uint32_t)) return parseFailed( "Invalid root signature, insufficient space for header."); @@ -273,9 +272,9 @@ Error DirectX::RootSignature::parse(StringRef Data) { Flags = support::endian::read<uint32_t, llvm::endianness::little>(Current); Current += sizeof(uint32_t); - assert(Current == Begin + RootParametersOffset); + assert(Current == PartData.begin() + RootParametersOffset); - ParametersHeaders.Data = Data.substr( + ParametersHeaders.Data = PartData.substr( RootParametersOffset, NumParameters * sizeof(dxbc::RootParameterHeader)); return Error::success(); diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp index 7d43e6c7f66b8..d94347a946123 100644 --- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp +++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp @@ -40,6 +40,8 @@ DXContainerYAML::RootSignatureYamlDesc::create( RootSigDesc.Version = Data.getVersion(); RootSigDesc.NumStaticSamplers = Data.getNumStaticSamplers(); RootSigDesc.StaticSamplersOffset = Data.getStaticSamplersOffset(); + RootSigDesc.NumRootParameters = Data.getNumRootParameters(); + RootSigDesc.RootParametersOffset = Data.getRootParametersOffset(); uint32_t Flags = Data.getFlags(); for (const auto &PH : Data.param_headers()) { @@ -51,13 +53,13 @@ DXContainerYAML::RootSignatureYamlDesc::create( return createStringError(std::errc::invalid_argument, "Invalid value for parameter type"); - NewP.Type = (dxbc::RootParameterType)PH.ParameterType; + NewP.Type = PH.ParameterType; if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility)) return createStringError(std::errc::invalid_argument, "Invalid value for shader visibility"); - NewP.Visibility = (dxbc::ShaderVisibility)PH.ShaderVisibility; + NewP.Visibility = PH.ShaderVisibility; llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr = Data.getParameter(PH); @@ -257,6 +259,8 @@ void MappingTraits<DXContainerYAML::Signature>::mapping( void MappingTraits<DXContainerYAML::RootSignatureYamlDesc>::mapping( IO &IO, DXContainerYAML::RootSignatureYamlDesc &S) { IO.mapRequired("Version", S.Version); + IO.mapRequired("NumRootParameters", S.NumRootParameters); + IO.mapRequired("RootParametersOffset", S.RootParametersOffset); IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers); IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset); IO.mapRequired("Parameters", S.Parameters); @@ -385,18 +389,6 @@ void ScalarEnumerationTraits<dxbc::SigComponentType>::enumeration( IO.enumCase(Value, E.Name.str().c_str(), E.Value); } -void ScalarEnumerationTraits<dxbc::RootParameterType>::enumeration( - IO &IO, dxbc::RootParameterType &Value) { - for (const auto &E : dxbc::getRootParameterTypes()) - IO.enumCase(Value, E.Name.str().c_str(), E.Value); -} - -void ScalarEnumerationTraits<dxbc::ShaderVisibility>::enumeration( - IO &IO, dxbc::ShaderVisibility &Value) { - for (const auto &E : dxbc::getShaderVisibility()) - IO.enumCase(Value, E.Name.str().c_str(), E.Value); -} - } // namespace yaml void DXContainerYAML::PSVInfo::mapInfoForVersion(yaml::IO &IO) { diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll index 1ca6ebb7ddab8..e81679732a5d8 100644 --- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll +++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll @@ -22,6 +22,8 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" } ; DXC-NEXT: Size: 24 ; DXC-NEXT: RootSignature: ; DXC-NEXT: Version: 2 +; DXC-NEXT: NumRootParameters: 0 +; DXC-NEXT: RootParametersOffset: 24 ; DXC-NEXT: NumStaticSamplers: 0 ; DXC-NEXT: StaticSamplersOffset: 0 ; DXC-NEXT: Parameters: [] diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml index 0d7902afdaa66..831664de2c9d9 100644 --- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml @@ -14,6 +14,8 @@ Parts: Size: 24 RootSignature: Version: 2 + NumRootParameters: 0 + RootParametersOffset: 24 NumStaticSamplers: 4 StaticSamplersOffset: 5 Parameters: [] @@ -24,6 +26,8 @@ Parts: # CHECK-NEXT: Size: 24 # CHECK-NEXT: RootSignature: # CHECK-NEXT: Version: 2 +# CHECK-NEXT: NumRootParameters: 0 +# CHECK-NEXT: RootParametersOffset: 24 # CHECK-NEXT: NumStaticSamplers: 0 # CHECK-NEXT: StaticSamplersOffset: 0 # CHECK-NEXT: Parameters: [] diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml new file mode 100644 index 0000000000000..091e70789d956 --- /dev/null +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml @@ -0,0 +1,29 @@ +# RUN: yaml2obj %s -o %t +# RUN: not obj2yaml 2>&1 %t | FileCheck %s -DFILE=%t + +# CHECK: Error reading file: [[FILE]]: Invalid value for parameter type + + +--- !dxcontainer +Header: + Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ] + Version: + Major: 1 + Minor: 0 + PartCount: 1 + PartOffsets: [ 60 ] +Parts: + - Name: RTS0 + Size: 80 + RootSignature: + Version: 2 + NumRootParameters: 2 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 64 + Parameters: + - ParameterType: 255 # INVALID + ShaderVisibility: 2 # Hull + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml new file mode 100644 index 0000000000000..1acaf6e4e08a4 --- /dev/null +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml @@ -0,0 +1,33 @@ +# RUN: yaml2obj %s -o %t +# RUN: not obj2yaml 2>&1 %t | FileCheck %s -DFILE=%t + +# CHECK: Error reading file: [[FILE]]: Invalid value for shader visibility + + +--- !dxcontainer +Header: + Hash: [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ] + Version: + Major: 1 + Minor: 0 + PartCount: 1 + PartOffsets: [ 60 ] +Parts: + - Name: RTS0 + Size: 80 + RootSignature: + Version: 2 + NumRootParameters: 2 + RootParametersOffset: 24 + NumStaticSamplers: 0 + StaticSamplersOffset: 64 + Parameters: + - ParameterType: 1 # Constants32Bit + ShaderVisibility: 255 # INVALID + Constants: + Num32BitValues: 21 + ShaderRegister: 22 + RegisterSpace: 23 + AllowInputAssemblerInputLayout: true + DenyGeometryShaderRootAccess: true diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml index 8d5ab5c1b0b23..d6316765e42fb 100644 --- a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml +++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml @@ -14,17 +14,19 @@ Parts: Size: 80 RootSignature: Version: 2 + NumRootParameters: 2 + RootParametersOffset: 24 NumStaticSamplers: 0 StaticSamplersOffset: 64 Parameters: - - ParameterType: Constants32Bit - ShaderVisibility: Hull + - ParameterType: 1 # Constants32Bit + ShaderVisibility: 2 # Hull Constants: Num32BitValues: 16 ShaderRegister: 15 RegisterSpace: 14 - - ParameterType: Constants32Bit - ShaderVisibility: Geometry + - ParameterType: 1 # Constants32Bit + ShaderVisibility: 4 # Geometry Constants: Num32BitValues: 21 ShaderRegister: 22 @@ -36,17 +38,19 @@ Parts: # CHECK-NEXT: Size: 80 # CHECK-NEXT: RootSignature: # CHECK-NEXT: Version: 2 +# CHECK-NEXT: NumRootParameters: 2 +# CHECK-NEXT: RootParametersOffset: 24 # CHECK-NEXT: NumStaticSamplers: 0 # CHECK-NEXT: StaticSamplersOffset: 0 # CHECK-NEXT: Parameters: -# CHECK-NEXT: - ParameterType: Constants32Bit -# CHECK-NEXT: ShaderVisibility: Hull +# CHECK-NEXT: - ParameterType: 1 +# CHECK-NEXT: ShaderVisibility: 2 # CHECK-NEXT: Constants: # CHECK-NEXT: Num32BitValues: 16 # CHECK-NEXT: RegisterSpace: 14 # CHECK-NEXT: ShaderRegister: 15 -# CHECK-NEXT: - ParameterType: Constants32Bit -# CHECK-NEXT: ShaderVisibility: Geometry +# CHECK-NEXT: - ParameterType: 1 +# CHECK-NEXT: ShaderVisibility: 4 # CHECK-NEXT: Constants: # CHECK-NEXT: Num32BitValues: 21 # CHECK-NEXT: RegisterSpace: 23 diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp index e8c299f4d5c08..bf32c07951520 100644 --- a/llvm/unittests/Object/DXContainerTest.cpp +++ b/llvm/unittests/Object/DXContainerTest.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "llvm/Object/DXContainer.h" -#include "../../tools/obj2yaml/dxcontainer2yaml.cpp" #include "llvm/ADT/StringRef.h" #include "llvm/BinaryFormat/Magic.h" #include "llvm/ObjectYAML/DXContainerYAML.h" @@ -908,44 +907,4 @@ TEST(RootSignature, ParseRootConstant) { ASSERT_EQ(Constants->RegisterSpace, 14u); ASSERT_EQ(Constants->Num32BitValues, 16u); } - { - uint8_t Buffer[] = { - 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, - 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, - 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, - 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, - 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00, - 0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00, - 0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00}; - - SmallString<256> Storage; - raw_svector_ostream OS(Storage); - EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)), - FailedWithMessage("Invalid value for parameter type")); - } - { - uint8_t Buffer[] = { - 0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f, - 0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00, - 0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, - 0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, - 0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, - 0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00, - 0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00}; - - SmallString<256> Storage; - raw_svector_ostream OS(Storage); - EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)), - FailedWithMessage("Invalid value for shader visibility")); - } } diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp index 3e5b79be2ccff..99ca161883498 100644 --- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp +++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp @@ -127,6 +127,8 @@ TEST(RootSignature, ParseRootFlags) { Size: 24 RootSignature: Version: 2 + NumRootParameters: 0 + RootParametersOffset: 24 NumStaticSamplers: 0 StaticSamplersOffset: 0 Parameters: [] @@ -165,11 +167,13 @@ TEST(RootSignature, ParseRootConstants) { Size: 89 RootSignature: Version: 2 + NumRootParameters: 1 + RootParametersOffset: 24 NumStaticSamplers: 0 StaticSamplersOffset: 56 Parameters: - - ParameterType: Constants32Bit - ShaderVisibility: Hull + - ParameterType: 1 + ShaderVisibility: 2 Constants: Num32BitValues: 16 ShaderRegister: 15 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits