[DXIL] Adding support to RootSignatureFlags in obj2yaml #122396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
backend:DirectX
objectyaml
llvm:analysis
|
@llvm/pr-subscribers-objectyaml @llvm/pr-subscribers-backend-directx Author: None (joaosaffran) ChangesThis PR adds:
Patch is 25.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122396.diff 14 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
index cb535ac14f1c61..89c5bffcdbb954 100644
--- a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
+++ b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
@@ -10,10 +10,12 @@
#define LLVM_ANALYSIS_DXILMETADATA_H
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Pass.h"
#include "llvm/Support/VersionTuple.h"
#include "llvm/TargetParser/Triple.h"
+#include <optional>
namespace llvm {
@@ -37,6 +39,7 @@ struct ModuleMetadataInfo {
Triple::EnvironmentType ShaderProfile{Triple::UnknownEnvironment};
VersionTuple ValidatorVersion{};
SmallVector<EntryProperties> EntryPropertyVec{};
+ std::optional<root_signature::VersionedRootSignatureDesc> RootSignatureDesc;
void print(raw_ostream &OS) const;
};
diff --git a/llvm/include/llvm/Analysis/DXILRootSignature.h b/llvm/include/llvm/Analysis/DXILRootSignature.h
new file mode 100644
index 00000000000000..cb3d6192f4404d
--- /dev/null
+++ b/llvm/include/llvm/Analysis/DXILRootSignature.h
@@ -0,0 +1,88 @@
+//===- DXILRootSignature.h - DXIL Root Signature helper objects -----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains helper objects for working with DXIL Root
+/// Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DIRECTX_HLSLROOTSIGNATURE_H
+#define LLVM_DIRECTX_HLSLROOTSIGNATURE_H
+
+#include "llvm/IR/Metadata.h"
+#include "llvm/Support/ScopedPrinter.h"
+namespace llvm {
+namespace dxil {
+namespace root_signature {
+
+enum class RootSignatureElementKind {
+ None = 0,
+ RootFlags = 1,
+ RootConstants = 2,
+ RootDescriptor = 3,
+ DescriptorTable = 4,
+ StaticSampler = 5
+};
+
+enum class RootSignatureVersion {
+ Version_1 = 1,
+ Version_1_0 = 1,
+ Version_1_1 = 2,
+ Version_1_2 = 3
+};
+
+enum RootSignatureFlags : uint32_t {
+ None = 0,
+ AllowInputAssemblerInputLayout = 0x1,
+ DenyVertexShaderRootAccess = 0x2,
+ DenyHullShaderRootAccess = 0x4,
+ DenyDomainShaderRootAccess = 0x8,
+ DenyGeometryShaderRootAccess = 0x10,
+ DenyPixelShaderRootAccess = 0x20,
+ AllowStreamOutput = 0x40,
+ LocalRootSignature = 0x80,
+ DenyAmplificationShaderRootAccess = 0x100,
+ DenyMeshShaderRootAccess = 0x200,
+ CBVSRVUAVHeapDirectlyIndexed = 0x400,
+ SamplerHeapDirectlyIndexed = 0x800,
+ AllowLowTierReservedHwCbLimit = 0x80000000,
+ ValidFlags = 0x80000fff
+};
+
+struct DxilRootSignatureDesc1_0 {
+ RootSignatureFlags Flags;
+};
+
+struct VersionedRootSignatureDesc {
+ RootSignatureVersion Version;
+ union {
+ DxilRootSignatureDesc1_0 Desc_1_0;
+ };
+
+ bool isPopulated();
+
+ void swapBytes();
+};
+
+class MetadataParser {
+public:
+ NamedMDNode *Root;
+ MetadataParser(NamedMDNode *Root) : Root(Root) {}
+
+ bool Parse(RootSignatureVersion Version, VersionedRootSignatureDesc *Desc);
+
+private:
+ bool ParseRootFlags(MDNode *RootFlagRoot, VersionedRootSignatureDesc *Desc);
+ bool ParseRootSignatureElement(MDNode *Element,
+ VersionedRootSignatureDesc *Desc);
+};
+} // namespace root_signature
+} // namespace dxil
+} // namespace llvm
+
+#endif // LLVM_DIRECTX_HLSLROOTSIGNATURE_H
diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
index 1aacbb2f65b27f..38b69228cd3975 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
+++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
@@ -4,6 +4,7 @@ CONTAINER_PART(DXIL)
CONTAINER_PART(SFI0)
CONTAINER_PART(HASH)
CONTAINER_PART(PSV0)
+CONTAINER_PART(RTS0)
CONTAINER_PART(ISG1)
CONTAINER_PART(OSG1)
CONTAINER_PART(PSG1)
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 19c83ba6c6e85d..9a6aa8224eddf4 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBufferRef.h"
@@ -287,6 +288,7 @@ class DXContainer {
std::optional<uint64_t> ShaderFeatureFlags;
std::optional<dxbc::ShaderHash> Hash;
std::optional<DirectX::PSVRuntimeInfo> PSVInfo;
+ std::optional<dxil::root_signature::VersionedRootSignatureDesc> RootSignature;
DirectX::Signature InputSignature;
DirectX::Signature OutputSignature;
DirectX::Signature PatchConstantSignature;
@@ -296,6 +298,7 @@ class DXContainer {
Error parseDXILHeader(StringRef Part);
Error parseShaderFeatureFlags(StringRef Part);
Error parseHash(StringRef Part);
+ Error parseRootSignature(StringRef Part);
Error parsePSVInfo(StringRef Part);
Error parseSignature(StringRef Part, DirectX::Signature &Array);
friend class PartIterator;
@@ -382,6 +385,11 @@ class DXContainer {
std::optional<dxbc::ShaderHash> getShaderHash() const { return Hash; }
+ std::optional<dxil::root_signature::VersionedRootSignatureDesc>
+ getRootSignature() const {
+ return RootSignature;
+ }
+
const std::optional<DirectX::PSVRuntimeInfo> &getPSVInfo() const {
return PSVInfo;
};
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 66ad057ab0e30f..e9da51f61c0a2b 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -16,6 +16,7 @@
#define LLVM_OBJECTYAML_DXCONTAINERYAML_H
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/ObjectYAML/YAML.h"
#include "llvm/Support/YAMLTraits.h"
@@ -149,6 +150,13 @@ struct Signature {
llvm::SmallVector<SignatureParameter> Parameters;
};
+struct RootSignature {
+ RootSignature() = default;
+
+ dxil::root_signature::RootSignatureVersion Version;
+ dxil::root_signature::RootSignatureFlags Flags;
+};
+
struct Part {
Part() = default;
Part(std::string N, uint32_t S) : Name(N), Size(S) {}
@@ -159,6 +167,7 @@ struct Part {
std::optional<ShaderHash> Hash;
std::optional<PSVInfo> Info;
std::optional<DXContainerYAML::Signature> Signature;
+ std::optional<DXContainerYAML::RootSignature> RootSignature;
};
struct Object {
@@ -241,6 +250,11 @@ template <> struct MappingTraits<DXContainerYAML::Signature> {
static void mapping(IO &IO, llvm::DXContainerYAML::Signature &El);
};
+template <> struct MappingTraits<DXContainerYAML::RootSignature> {
+ static void mapping(IO &IO,
+ llvm::DXContainerYAML::RootSignature &RootSignature);
+};
+
} // namespace yaml
} // namespace llvm
diff --git a/llvm/lib/Analysis/CMakeLists.txt b/llvm/lib/Analysis/CMakeLists.txt
index 0db5b80f336cb5..8875ddd34fe56c 100644
--- a/llvm/lib/Analysis/CMakeLists.txt
+++ b/llvm/lib/Analysis/CMakeLists.txt
@@ -62,6 +62,7 @@ add_llvm_component_library(LLVMAnalysis
DominanceFrontier.cpp
DXILResource.cpp
DXILMetadataAnalysis.cpp
+ DXILRootSignature.cpp
FunctionPropertiesAnalysis.cpp
GlobalsModRef.cpp
GuardUtils.cpp
diff --git a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
index a7f666a3f8b48f..3bd60bfe203f49 100644
--- a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
+++ b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
@@ -10,12 +10,15 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
+#include <memory>
#define DEBUG_TYPE "dxil-metadata-analysis"
@@ -28,6 +31,7 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
MMDAI.DXILVersion = TT.getDXILVersion();
MMDAI.ShaderModelVersion = TT.getOSVersion();
MMDAI.ShaderProfile = TT.getEnvironment();
+
NamedMDNode *ValidatorVerNode = M.getNamedMetadata("dx.valver");
if (ValidatorVerNode) {
auto *ValVerMD = cast<MDNode>(ValidatorVerNode->getOperand(0));
@@ -37,6 +41,19 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
VersionTuple(MajorMD->getZExtValue(), MinorMD->getZExtValue());
}
+ NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures");
+ if (RootSignatureNode) {
+ auto RootSignatureParser =
+ root_signature::MetadataParser(RootSignatureNode);
+
+ root_signature::VersionedRootSignatureDesc Desc;
+
+ RootSignatureParser.Parse(root_signature::RootSignatureVersion::Version_1,
+ &Desc);
+
+ MMDAI.RootSignatureDesc = Desc;
+ }
+
// For all HLSL Shader functions
for (auto &F : M.functions()) {
if (!F.hasFnAttribute("hlsl.shader"))
diff --git a/llvm/lib/Analysis/DXILRootSignature.cpp b/llvm/lib/Analysis/DXILRootSignature.cpp
new file mode 100644
index 00000000000000..fce97eb27cf8f8
--- /dev/null
+++ b/llvm/lib/Analysis/DXILRootSignature.cpp
@@ -0,0 +1,110 @@
+//===- DXILRootSignature.cpp - DXIL Root Signature helper objects
+//-----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains the parsing logic to extract root signature data
+/// from LLVM IR metadata.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/DXILRootSignature.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/Support/ErrorHandling.h"
+#include <cassert>
+
+namespace llvm {
+namespace dxil {
+
+bool root_signature::MetadataParser::Parse(RootSignatureVersion Version,
+ VersionedRootSignatureDesc *Desc) {
+ Desc->Version = Version;
+ bool HasError = false;
+
+ for (unsigned int Sid = 0; Sid < Root->getNumOperands(); Sid++) {
+ // This should be an if, for error handling
+ MDNode *Node = cast<MDNode>(Root->getOperand(Sid));
+
+ // Not sure what use this for...
+ Metadata *Func = Node->getOperand(0).get();
+
+ // This should be an if, for error handling
+ MDNode *Elements = cast<MDNode>(Node->getOperand(1).get());
+
+ for (unsigned int Eid = 0; Eid < Elements->getNumOperands(); Eid++) {
+ MDNode *Element = cast<MDNode>(Elements->getOperand(Eid));
+
+ HasError = HasError || ParseRootSignatureElement(Element, Desc);
+ }
+ }
+ return HasError;
+}
+
+bool root_signature::MetadataParser::ParseRootFlags(
+ MDNode *RootFlagNode, VersionedRootSignatureDesc *Desc) {
+
+ assert(RootFlagNode->getNumOperands() == 2 &&
+ "Invalid format for RootFlag Element");
+ auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1));
+ auto Value = (RootSignatureFlags)Flag->getZExtValue();
+
+ if ((Value & ~RootSignatureFlags::ValidFlags) != RootSignatureFlags::None)
+ return true;
+
+ switch (Desc->Version) {
+
+ case RootSignatureVersion::Version_1:
+ Desc->Desc_1_0.Flags = (RootSignatureFlags)Value;
+ break;
+ case RootSignatureVersion::Version_1_1:
+ case RootSignatureVersion::Version_1_2:
+ llvm_unreachable("Not implemented yet");
+ break;
+ }
+ return false;
+}
+
+bool root_signature::MetadataParser::ParseRootSignatureElement(
+ MDNode *Element, VersionedRootSignatureDesc *Desc) {
+ MDString *ElementText = cast<MDString>(Element->getOperand(0));
+
+ assert(ElementText != nullptr && "First preoperty of element is not ");
+
+ RootSignatureElementKind ElementKind =
+ StringSwitch<RootSignatureElementKind>(ElementText->getString())
+ .Case("RootFlags", RootSignatureElementKind::RootFlags)
+ .Case("RootConstants", RootSignatureElementKind::RootConstants)
+ .Case("RootCBV", RootSignatureElementKind::RootDescriptor)
+ .Case("RootSRV", RootSignatureElementKind::RootDescriptor)
+ .Case("RootUAV", RootSignatureElementKind::RootDescriptor)
+ .Case("Sampler", RootSignatureElementKind::RootDescriptor)
+ .Case("DescriptorTable", RootSignatureElementKind::DescriptorTable)
+ .Case("StaticSampler", RootSignatureElementKind::StaticSampler)
+ .Default(RootSignatureElementKind::None);
+
+ switch (ElementKind) {
+
+ case RootSignatureElementKind::RootFlags: {
+ return ParseRootFlags(Element, Desc);
+ break;
+ }
+
+ case RootSignatureElementKind::RootConstants:
+ case RootSignatureElementKind::RootDescriptor:
+ case RootSignatureElementKind::DescriptorTable:
+ case RootSignatureElementKind::StaticSampler:
+ case RootSignatureElementKind::None:
+ llvm_unreachable("Not Implemented yet");
+ break;
+ }
+
+ return true;
+}
+} // namespace dxil
+} // namespace llvm
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 3b1a6203a1f8fc..f50f68df88ec2a 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -7,9 +7,11 @@
//===----------------------------------------------------------------------===//
#include "llvm/Object/DXContainer.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Object/Error.h"
#include "llvm/Support/Alignment.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
using namespace llvm;
@@ -92,6 +94,14 @@ Error DXContainer::parseHash(StringRef Part) {
return Error::success();
}
+Error DXContainer::parseRootSignature(StringRef Part) {
+ dxil::root_signature::VersionedRootSignatureDesc Desc;
+ if (Error Err = readStruct(Part, Part.begin(), Desc))
+ return Err;
+ RootSignature = Desc;
+ return Error::success();
+}
+
Error DXContainer::parsePSVInfo(StringRef Part) {
if (PSVInfo)
return parseFailed("More than one PSV0 part is present in the file");
@@ -192,6 +202,11 @@ Error DXContainer::parsePartOffsets() {
return Err;
break;
case dxbc::PartType::Unknown:
+ break;
+ case dxbc::PartType::RTS0:
+ if (Error Err = parseRootSignature(PartData))
+ return Err;
+
break;
}
}
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index 175f1a12f93145..905d409562ff45 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -11,6 +11,7 @@
///
//===----------------------------------------------------------------------===//
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/MC/DXContainerPSVInfo.h"
#include "llvm/ObjectYAML/ObjectYAML.h"
@@ -261,6 +262,12 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
}
case dxbc::PartType::Unknown:
break; // Skip any handling for unrecognized parts.
+ case dxbc::PartType::RTS0:
+ if (!P.RootSignature.has_value())
+ continue;
+ OS.write(reinterpret_cast<const char *>(&P.RootSignature),
+ sizeof(dxil::root_signature::VersionedRootSignatureDesc));
+ break;
}
uint64_t BytesWritten = OS.tell() - DataStart;
RollingOffset += BytesWritten;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 5dee1221b27c01..eab3fcc5936f85 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -13,6 +13,7 @@
#include "llvm/ObjectYAML/DXContainerYAML.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -188,6 +189,12 @@ void MappingTraits<DXContainerYAML::Signature>::mapping(
IO.mapRequired("Parameters", S.Parameters);
}
+void MappingTraits<DXContainerYAML::RootSignature>::mapping(
+ IO &IO, DXContainerYAML::RootSignature &S) {
+ IO.mapRequired("Version", S.Version);
+ IO.mapRequired("Flags", S.Flags);
+}
+
void MappingTraits<DXContainerYAML::Part>::mapping(IO &IO,
DXContainerYAML::Part &P) {
IO.mapRequired("Name", P.Name);
@@ -197,6 +204,7 @@ void MappingTraits<DXContainerYAML::Part>::mapping(IO &IO,
IO.mapOptional("Hash", P.Hash);
IO.mapOptional("PSVInfo", P.Info);
IO.mapOptional("Signature", P.Signature);
+ IO.mapOptional("RootSignature", P.RootSignature);
}
void MappingTraits<DXContainerYAML::Object>::mapping(
@@ -290,6 +298,66 @@ void ScalarEnumerationTraits<dxbc::SigComponentType>::enumeration(
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}
+template <>
+struct llvm::yaml::ScalarEnumerationTraits<
+ dxil::root_signature::RootSignatureVersion> {
+ static void enumeration(IO &io,
+ dxil::root_signature::RootSignatureVersion &Val) {
+ io.enumCase(Val, "1.0",
+ dxil::root_signature::RootSignatureVersion::Version_1);
+ io.enumCase(Val, "1.0",
+ dxil::root_signature::RootSignatureVersion::Version_1_0);
+ io.enumCase(Val, "1.1",
+ dxil::root_signature::RootSignatureVersion::Version_1_1);
+ io.enumCase(Val, "1.2",
+ dxil::root_signature::RootSignatureVersion::Version_1_2);
+ }
+};
+
+template <>
+struct llvm::yaml::ScalarEnumerationTraits<
+ dxil::root_signature::RootSignatureFlags> {
+ static void enumeration(IO &io,
+ dxil::root_signature::RootSignatureFlags &Val) {
+ io.enumCase(Val, "AllowInputAssemblerInputLayout",
+ dxil::root_signature::RootSignatureFlags::
+ AllowInputAssemblerInputLayout);
+ io.enumCase(
+ Val, "DenyVertexShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyVertexShaderRootAccess);
+ io.enumCase(
+ Val, "DenyHullShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyHullShaderRootAccess);
+ io.enumCase(
+ Val, "DenyDomainShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyDomainShaderRootAccess);
+ io.enumCase(
+ Val, "DenyGeometryShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyGeometryShaderRootAccess);
+ io.enumCase(
+ Val, "DenyPixelShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyPixelShaderRootAccess);
+ io.enumCase(Val, "AllowStreamOutput",
+ dxil::root_signature::RootSignatureFlags::AllowStreamOutput);
+ io.enumCase(Val, "LocalRootSignature",
+ dxil::root_signature::RootSignatureFlags::LocalRootSignature);
+ io.enumCase(Val, "DenyAmplificationShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::
+ DenyAmplificationShaderRootAccess);
+ io.enumCase(
+ Val, "DenyMeshShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyMeshShaderRootAccess);
+ io.enumCase(
+ Val, "CBVSRVUAVHeapDirectlyIndexed",
+ dxil::root_signature::RootSignatureFlags::CBVSRVUAVHeapDirectlyIndexed);
+ io.enumCase(
+ Val, "SamplerHeapDirectlyIndexed",
+ dxil::root_signature::RootSignatureFlags::SamplerHeapDirectlyIndexed);
+ io.enumCase(Val, "AllowLowTierReservedHwCbLimit",
+ dxil::root_signature::RootSignatureFlags::
+ AllowLowTierReservedHwCbLimit);
+ }
+};
} // namespace yaml
void DXContainerYAML::PSVInfo::mapInfoForVersion(yaml::IO &IO) {
diff --git a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
index 7a0bd6a7c88692..e3174d600e6534 100644
--- a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
+++ b/llvm/lib/Target/Direct...
[truncated]
|
|
@llvm/pr-subscribers-llvm-analysis Author: None (joaosaffran) ChangesThis PR adds:
Patch is 25.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/122396.diff 14 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
index cb535ac14f1c61..89c5bffcdbb954 100644
--- a/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
+++ b/llvm/include/llvm/Analysis/DXILMetadataAnalysis.h
@@ -10,10 +10,12 @@
#define LLVM_ANALYSIS_DXILMETADATA_H
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Pass.h"
#include "llvm/Support/VersionTuple.h"
#include "llvm/TargetParser/Triple.h"
+#include <optional>
namespace llvm {
@@ -37,6 +39,7 @@ struct ModuleMetadataInfo {
Triple::EnvironmentType ShaderProfile{Triple::UnknownEnvironment};
VersionTuple ValidatorVersion{};
SmallVector<EntryProperties> EntryPropertyVec{};
+ std::optional<root_signature::VersionedRootSignatureDesc> RootSignatureDesc;
void print(raw_ostream &OS) const;
};
diff --git a/llvm/include/llvm/Analysis/DXILRootSignature.h b/llvm/include/llvm/Analysis/DXILRootSignature.h
new file mode 100644
index 00000000000000..cb3d6192f4404d
--- /dev/null
+++ b/llvm/include/llvm/Analysis/DXILRootSignature.h
@@ -0,0 +1,88 @@
+//===- DXILRootSignature.h - DXIL Root Signature helper objects -----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains helper objects for working with DXIL Root
+/// Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_DIRECTX_HLSLROOTSIGNATURE_H
+#define LLVM_DIRECTX_HLSLROOTSIGNATURE_H
+
+#include "llvm/IR/Metadata.h"
+#include "llvm/Support/ScopedPrinter.h"
+namespace llvm {
+namespace dxil {
+namespace root_signature {
+
+enum class RootSignatureElementKind {
+ None = 0,
+ RootFlags = 1,
+ RootConstants = 2,
+ RootDescriptor = 3,
+ DescriptorTable = 4,
+ StaticSampler = 5
+};
+
+enum class RootSignatureVersion {
+ Version_1 = 1,
+ Version_1_0 = 1,
+ Version_1_1 = 2,
+ Version_1_2 = 3
+};
+
+enum RootSignatureFlags : uint32_t {
+ None = 0,
+ AllowInputAssemblerInputLayout = 0x1,
+ DenyVertexShaderRootAccess = 0x2,
+ DenyHullShaderRootAccess = 0x4,
+ DenyDomainShaderRootAccess = 0x8,
+ DenyGeometryShaderRootAccess = 0x10,
+ DenyPixelShaderRootAccess = 0x20,
+ AllowStreamOutput = 0x40,
+ LocalRootSignature = 0x80,
+ DenyAmplificationShaderRootAccess = 0x100,
+ DenyMeshShaderRootAccess = 0x200,
+ CBVSRVUAVHeapDirectlyIndexed = 0x400,
+ SamplerHeapDirectlyIndexed = 0x800,
+ AllowLowTierReservedHwCbLimit = 0x80000000,
+ ValidFlags = 0x80000fff
+};
+
+struct DxilRootSignatureDesc1_0 {
+ RootSignatureFlags Flags;
+};
+
+struct VersionedRootSignatureDesc {
+ RootSignatureVersion Version;
+ union {
+ DxilRootSignatureDesc1_0 Desc_1_0;
+ };
+
+ bool isPopulated();
+
+ void swapBytes();
+};
+
+class MetadataParser {
+public:
+ NamedMDNode *Root;
+ MetadataParser(NamedMDNode *Root) : Root(Root) {}
+
+ bool Parse(RootSignatureVersion Version, VersionedRootSignatureDesc *Desc);
+
+private:
+ bool ParseRootFlags(MDNode *RootFlagRoot, VersionedRootSignatureDesc *Desc);
+ bool ParseRootSignatureElement(MDNode *Element,
+ VersionedRootSignatureDesc *Desc);
+};
+} // namespace root_signature
+} // namespace dxil
+} // namespace llvm
+
+#endif // LLVM_DIRECTX_HLSLROOTSIGNATURE_H
diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
index 1aacbb2f65b27f..38b69228cd3975 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
+++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
@@ -4,6 +4,7 @@ CONTAINER_PART(DXIL)
CONTAINER_PART(SFI0)
CONTAINER_PART(HASH)
CONTAINER_PART(PSV0)
+CONTAINER_PART(RTS0)
CONTAINER_PART(ISG1)
CONTAINER_PART(OSG1)
CONTAINER_PART(PSG1)
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 19c83ba6c6e85d..9a6aa8224eddf4 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -17,6 +17,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBufferRef.h"
@@ -287,6 +288,7 @@ class DXContainer {
std::optional<uint64_t> ShaderFeatureFlags;
std::optional<dxbc::ShaderHash> Hash;
std::optional<DirectX::PSVRuntimeInfo> PSVInfo;
+ std::optional<dxil::root_signature::VersionedRootSignatureDesc> RootSignature;
DirectX::Signature InputSignature;
DirectX::Signature OutputSignature;
DirectX::Signature PatchConstantSignature;
@@ -296,6 +298,7 @@ class DXContainer {
Error parseDXILHeader(StringRef Part);
Error parseShaderFeatureFlags(StringRef Part);
Error parseHash(StringRef Part);
+ Error parseRootSignature(StringRef Part);
Error parsePSVInfo(StringRef Part);
Error parseSignature(StringRef Part, DirectX::Signature &Array);
friend class PartIterator;
@@ -382,6 +385,11 @@ class DXContainer {
std::optional<dxbc::ShaderHash> getShaderHash() const { return Hash; }
+ std::optional<dxil::root_signature::VersionedRootSignatureDesc>
+ getRootSignature() const {
+ return RootSignature;
+ }
+
const std::optional<DirectX::PSVRuntimeInfo> &getPSVInfo() const {
return PSVInfo;
};
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 66ad057ab0e30f..e9da51f61c0a2b 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -16,6 +16,7 @@
#define LLVM_OBJECTYAML_DXCONTAINERYAML_H
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/ObjectYAML/YAML.h"
#include "llvm/Support/YAMLTraits.h"
@@ -149,6 +150,13 @@ struct Signature {
llvm::SmallVector<SignatureParameter> Parameters;
};
+struct RootSignature {
+ RootSignature() = default;
+
+ dxil::root_signature::RootSignatureVersion Version;
+ dxil::root_signature::RootSignatureFlags Flags;
+};
+
struct Part {
Part() = default;
Part(std::string N, uint32_t S) : Name(N), Size(S) {}
@@ -159,6 +167,7 @@ struct Part {
std::optional<ShaderHash> Hash;
std::optional<PSVInfo> Info;
std::optional<DXContainerYAML::Signature> Signature;
+ std::optional<DXContainerYAML::RootSignature> RootSignature;
};
struct Object {
@@ -241,6 +250,11 @@ template <> struct MappingTraits<DXContainerYAML::Signature> {
static void mapping(IO &IO, llvm::DXContainerYAML::Signature &El);
};
+template <> struct MappingTraits<DXContainerYAML::RootSignature> {
+ static void mapping(IO &IO,
+ llvm::DXContainerYAML::RootSignature &RootSignature);
+};
+
} // namespace yaml
} // namespace llvm
diff --git a/llvm/lib/Analysis/CMakeLists.txt b/llvm/lib/Analysis/CMakeLists.txt
index 0db5b80f336cb5..8875ddd34fe56c 100644
--- a/llvm/lib/Analysis/CMakeLists.txt
+++ b/llvm/lib/Analysis/CMakeLists.txt
@@ -62,6 +62,7 @@ add_llvm_component_library(LLVMAnalysis
DominanceFrontier.cpp
DXILResource.cpp
DXILMetadataAnalysis.cpp
+ DXILRootSignature.cpp
FunctionPropertiesAnalysis.cpp
GlobalsModRef.cpp
GuardUtils.cpp
diff --git a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
index a7f666a3f8b48f..3bd60bfe203f49 100644
--- a/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
+++ b/llvm/lib/Analysis/DXILMetadataAnalysis.cpp
@@ -10,12 +10,15 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Module.h"
#include "llvm/InitializePasses.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
+#include <memory>
#define DEBUG_TYPE "dxil-metadata-analysis"
@@ -28,6 +31,7 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
MMDAI.DXILVersion = TT.getDXILVersion();
MMDAI.ShaderModelVersion = TT.getOSVersion();
MMDAI.ShaderProfile = TT.getEnvironment();
+
NamedMDNode *ValidatorVerNode = M.getNamedMetadata("dx.valver");
if (ValidatorVerNode) {
auto *ValVerMD = cast<MDNode>(ValidatorVerNode->getOperand(0));
@@ -37,6 +41,19 @@ static ModuleMetadataInfo collectMetadataInfo(Module &M) {
VersionTuple(MajorMD->getZExtValue(), MinorMD->getZExtValue());
}
+ NamedMDNode *RootSignatureNode = M.getNamedMetadata("dx.rootsignatures");
+ if (RootSignatureNode) {
+ auto RootSignatureParser =
+ root_signature::MetadataParser(RootSignatureNode);
+
+ root_signature::VersionedRootSignatureDesc Desc;
+
+ RootSignatureParser.Parse(root_signature::RootSignatureVersion::Version_1,
+ &Desc);
+
+ MMDAI.RootSignatureDesc = Desc;
+ }
+
// For all HLSL Shader functions
for (auto &F : M.functions()) {
if (!F.hasFnAttribute("hlsl.shader"))
diff --git a/llvm/lib/Analysis/DXILRootSignature.cpp b/llvm/lib/Analysis/DXILRootSignature.cpp
new file mode 100644
index 00000000000000..fce97eb27cf8f8
--- /dev/null
+++ b/llvm/lib/Analysis/DXILRootSignature.cpp
@@ -0,0 +1,110 @@
+//===- DXILRootSignature.cpp - DXIL Root Signature helper objects
+//-----------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains the parsing logic to extract root signature data
+/// from LLVM IR metadata.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/DXILRootSignature.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/Support/ErrorHandling.h"
+#include <cassert>
+
+namespace llvm {
+namespace dxil {
+
+bool root_signature::MetadataParser::Parse(RootSignatureVersion Version,
+ VersionedRootSignatureDesc *Desc) {
+ Desc->Version = Version;
+ bool HasError = false;
+
+ for (unsigned int Sid = 0; Sid < Root->getNumOperands(); Sid++) {
+ // This should be an if, for error handling
+ MDNode *Node = cast<MDNode>(Root->getOperand(Sid));
+
+ // Not sure what use this for...
+ Metadata *Func = Node->getOperand(0).get();
+
+ // This should be an if, for error handling
+ MDNode *Elements = cast<MDNode>(Node->getOperand(1).get());
+
+ for (unsigned int Eid = 0; Eid < Elements->getNumOperands(); Eid++) {
+ MDNode *Element = cast<MDNode>(Elements->getOperand(Eid));
+
+ HasError = HasError || ParseRootSignatureElement(Element, Desc);
+ }
+ }
+ return HasError;
+}
+
+bool root_signature::MetadataParser::ParseRootFlags(
+ MDNode *RootFlagNode, VersionedRootSignatureDesc *Desc) {
+
+ assert(RootFlagNode->getNumOperands() == 2 &&
+ "Invalid format for RootFlag Element");
+ auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1));
+ auto Value = (RootSignatureFlags)Flag->getZExtValue();
+
+ if ((Value & ~RootSignatureFlags::ValidFlags) != RootSignatureFlags::None)
+ return true;
+
+ switch (Desc->Version) {
+
+ case RootSignatureVersion::Version_1:
+ Desc->Desc_1_0.Flags = (RootSignatureFlags)Value;
+ break;
+ case RootSignatureVersion::Version_1_1:
+ case RootSignatureVersion::Version_1_2:
+ llvm_unreachable("Not implemented yet");
+ break;
+ }
+ return false;
+}
+
+bool root_signature::MetadataParser::ParseRootSignatureElement(
+ MDNode *Element, VersionedRootSignatureDesc *Desc) {
+ MDString *ElementText = cast<MDString>(Element->getOperand(0));
+
+ assert(ElementText != nullptr && "First preoperty of element is not ");
+
+ RootSignatureElementKind ElementKind =
+ StringSwitch<RootSignatureElementKind>(ElementText->getString())
+ .Case("RootFlags", RootSignatureElementKind::RootFlags)
+ .Case("RootConstants", RootSignatureElementKind::RootConstants)
+ .Case("RootCBV", RootSignatureElementKind::RootDescriptor)
+ .Case("RootSRV", RootSignatureElementKind::RootDescriptor)
+ .Case("RootUAV", RootSignatureElementKind::RootDescriptor)
+ .Case("Sampler", RootSignatureElementKind::RootDescriptor)
+ .Case("DescriptorTable", RootSignatureElementKind::DescriptorTable)
+ .Case("StaticSampler", RootSignatureElementKind::StaticSampler)
+ .Default(RootSignatureElementKind::None);
+
+ switch (ElementKind) {
+
+ case RootSignatureElementKind::RootFlags: {
+ return ParseRootFlags(Element, Desc);
+ break;
+ }
+
+ case RootSignatureElementKind::RootConstants:
+ case RootSignatureElementKind::RootDescriptor:
+ case RootSignatureElementKind::DescriptorTable:
+ case RootSignatureElementKind::StaticSampler:
+ case RootSignatureElementKind::None:
+ llvm_unreachable("Not Implemented yet");
+ break;
+ }
+
+ return true;
+}
+} // namespace dxil
+} // namespace llvm
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 3b1a6203a1f8fc..f50f68df88ec2a 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -7,9 +7,11 @@
//===----------------------------------------------------------------------===//
#include "llvm/Object/DXContainer.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Object/Error.h"
#include "llvm/Support/Alignment.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FormatVariadic.h"
using namespace llvm;
@@ -92,6 +94,14 @@ Error DXContainer::parseHash(StringRef Part) {
return Error::success();
}
+Error DXContainer::parseRootSignature(StringRef Part) {
+ dxil::root_signature::VersionedRootSignatureDesc Desc;
+ if (Error Err = readStruct(Part, Part.begin(), Desc))
+ return Err;
+ RootSignature = Desc;
+ return Error::success();
+}
+
Error DXContainer::parsePSVInfo(StringRef Part) {
if (PSVInfo)
return parseFailed("More than one PSV0 part is present in the file");
@@ -192,6 +202,11 @@ Error DXContainer::parsePartOffsets() {
return Err;
break;
case dxbc::PartType::Unknown:
+ break;
+ case dxbc::PartType::RTS0:
+ if (Error Err = parseRootSignature(PartData))
+ return Err;
+
break;
}
}
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index 175f1a12f93145..905d409562ff45 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -11,6 +11,7 @@
///
//===----------------------------------------------------------------------===//
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/MC/DXContainerPSVInfo.h"
#include "llvm/ObjectYAML/ObjectYAML.h"
@@ -261,6 +262,12 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
}
case dxbc::PartType::Unknown:
break; // Skip any handling for unrecognized parts.
+ case dxbc::PartType::RTS0:
+ if (!P.RootSignature.has_value())
+ continue;
+ OS.write(reinterpret_cast<const char *>(&P.RootSignature),
+ sizeof(dxil::root_signature::VersionedRootSignatureDesc));
+ break;
}
uint64_t BytesWritten = OS.tell() - DataStart;
RollingOffset += BytesWritten;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 5dee1221b27c01..eab3fcc5936f85 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -13,6 +13,7 @@
#include "llvm/ObjectYAML/DXContainerYAML.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Analysis/DXILRootSignature.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -188,6 +189,12 @@ void MappingTraits<DXContainerYAML::Signature>::mapping(
IO.mapRequired("Parameters", S.Parameters);
}
+void MappingTraits<DXContainerYAML::RootSignature>::mapping(
+ IO &IO, DXContainerYAML::RootSignature &S) {
+ IO.mapRequired("Version", S.Version);
+ IO.mapRequired("Flags", S.Flags);
+}
+
void MappingTraits<DXContainerYAML::Part>::mapping(IO &IO,
DXContainerYAML::Part &P) {
IO.mapRequired("Name", P.Name);
@@ -197,6 +204,7 @@ void MappingTraits<DXContainerYAML::Part>::mapping(IO &IO,
IO.mapOptional("Hash", P.Hash);
IO.mapOptional("PSVInfo", P.Info);
IO.mapOptional("Signature", P.Signature);
+ IO.mapOptional("RootSignature", P.RootSignature);
}
void MappingTraits<DXContainerYAML::Object>::mapping(
@@ -290,6 +298,66 @@ void ScalarEnumerationTraits<dxbc::SigComponentType>::enumeration(
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}
+template <>
+struct llvm::yaml::ScalarEnumerationTraits<
+ dxil::root_signature::RootSignatureVersion> {
+ static void enumeration(IO &io,
+ dxil::root_signature::RootSignatureVersion &Val) {
+ io.enumCase(Val, "1.0",
+ dxil::root_signature::RootSignatureVersion::Version_1);
+ io.enumCase(Val, "1.0",
+ dxil::root_signature::RootSignatureVersion::Version_1_0);
+ io.enumCase(Val, "1.1",
+ dxil::root_signature::RootSignatureVersion::Version_1_1);
+ io.enumCase(Val, "1.2",
+ dxil::root_signature::RootSignatureVersion::Version_1_2);
+ }
+};
+
+template <>
+struct llvm::yaml::ScalarEnumerationTraits<
+ dxil::root_signature::RootSignatureFlags> {
+ static void enumeration(IO &io,
+ dxil::root_signature::RootSignatureFlags &Val) {
+ io.enumCase(Val, "AllowInputAssemblerInputLayout",
+ dxil::root_signature::RootSignatureFlags::
+ AllowInputAssemblerInputLayout);
+ io.enumCase(
+ Val, "DenyVertexShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyVertexShaderRootAccess);
+ io.enumCase(
+ Val, "DenyHullShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyHullShaderRootAccess);
+ io.enumCase(
+ Val, "DenyDomainShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyDomainShaderRootAccess);
+ io.enumCase(
+ Val, "DenyGeometryShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyGeometryShaderRootAccess);
+ io.enumCase(
+ Val, "DenyPixelShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyPixelShaderRootAccess);
+ io.enumCase(Val, "AllowStreamOutput",
+ dxil::root_signature::RootSignatureFlags::AllowStreamOutput);
+ io.enumCase(Val, "LocalRootSignature",
+ dxil::root_signature::RootSignatureFlags::LocalRootSignature);
+ io.enumCase(Val, "DenyAmplificationShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::
+ DenyAmplificationShaderRootAccess);
+ io.enumCase(
+ Val, "DenyMeshShaderRootAccess",
+ dxil::root_signature::RootSignatureFlags::DenyMeshShaderRootAccess);
+ io.enumCase(
+ Val, "CBVSRVUAVHeapDirectlyIndexed",
+ dxil::root_signature::RootSignatureFlags::CBVSRVUAVHeapDirectlyIndexed);
+ io.enumCase(
+ Val, "SamplerHeapDirectlyIndexed",
+ dxil::root_signature::RootSignatureFlags::SamplerHeapDirectlyIndexed);
+ io.enumCase(Val, "AllowLowTierReservedHwCbLimit",
+ dxil::root_signature::RootSignatureFlags::
+ AllowLowTierReservedHwCbLimit);
+ }
+};
} // namespace yaml
void DXContainerYAML::PSVInfo::mapInfoForVersion(yaml::IO &IO) {
diff --git a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
index 7a0bd6a7c88692..e3174d600e6534 100644
--- a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
+++ b/llvm/lib/Target/Direct...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvm-beanz
reviewed
Jan 10, 2025
There was a problem hiding this comment.
In the past when we've made changes to support emitting new parts of the container file we've broken the changes up into multiple PRs that land separately. Usually the first PR adds parsing and YAML tooling (with YAML roundtrip tests), and the second PR adds support for generating the data from IR and emitting the final file.
I believe this PR would benefit from being split up that way to make it easier to review.
llvm-beanz
requested changes
Jan 11, 2025
There was a problem hiding this comment.
There is a lot about this PR that needs to change. I'll work through writing detailed feedback next week but some high level things:
- Splitting up the change will make it a lot easier to review (I commented this separately too).
- There are no tests of the yaml2obj support.
- It doesn't look like you're handling endianness on the encoding.
- The code structure is all off.
ObjectYAML cannot and should not depend on the IR analysis library. The code for encoding binary structures should be part of the MC library not the analysis library. Take a look at how the code for the PSV0 data is structured because this should be closer to that.
joaosaffran
force-pushed
the
roosignatures/backend
branch
from
988e7b0 to
8adb678
Compare
joaosaffran
changed the title
added 2 commits
January 13, 2025 22:35
llvm-beanz
reviewed
Jan 14, 2025
| void MappingTraits<DXContainerYAML::RootSignatureDesc>::mapping( | ||
| IO &IO, DXContainerYAML::RootSignatureDesc &S) { | ||
| IO.mapRequired("Version", S.Version); | ||
| IO.mapRequired("Flags", S.Flags); |
There was a problem hiding this comment.
You might look at how we did the ShaderFeatureFlags. That approach makes the flags print nicer and more human readable in the YAML output.
In going that route the one thing I would probably do differently is making each flag "optional" rather than required in the yaml with a default value as false. That will make the printing more concise.
| FileSize: 1672 | ||
| PartCount: 7 | ||
| PartOffsets: [ 60, 1496, 1512, 1540, 1556, 1572, 1588 ] | ||
| Parts: |
There was a problem hiding this comment.
It is valid to have a root signature in a container by itself, so you shouldn't need a bunch of different parts. This test is an example of generating a root-signature-only container:
https://github.com/microsoft/DirectXShaderCompiler/blob/main/tools/clang/test/DXC/local_rs.hlsl
inbelic
reviewed
Jan 16, 2025
| if (RS && RS.has_value()) | ||
| NewPart.RootSignature = DXContainerYAML::RootSignatureDesc(*RS); | ||
| break; | ||
| break; |
| @@ -153,6 +153,12 @@ dumpDXContainer(MemoryBufferRef Source) { | |||
| break; | |||
| case dxbc::PartType::Unknown: | |||
| break; | |||
| case dxbc::PartType::RTS0: | |||
| std::optional<dxbc::RootSignatureDesc> RS = Container.getRootSignature(); | |||
| if (RS && RS.has_value()) | |||
There was a problem hiding this comment.
Suggested change
| if (RS && RS.has_value()) | |
| if (RS.has_value()) |
llvm/lib/Object/DXContainer.cpp
Outdated
| case dxbc::PartType::RTS0: | ||
| if (Error Err = parseRootSignature(PartData)) | ||
| return Err; | ||
|
|
7 tasks
damyanp
reviewed
Feb 4, 2025
Comment on lines
56
to
64
| #ifdef ROOT_PARAMETER | ||
|
|
||
| ROOT_PARAMETER(DescriptorTable) | ||
| ROOT_PARAMETER(Constants32Bit) | ||
| ROOT_PARAMETER(CBV) | ||
| ROOT_PARAMETER(SRV) | ||
| ROOT_PARAMETER(UAV) | ||
| #undef ROOT_PARAMETER | ||
| #endif // ROOT_PARAMETER |
There was a problem hiding this comment.
I don't see any uses of ROOT_PARAMETER or SHADER_VISIBILITY. Can we hold off adding them until they're used please?
|
|
||
| #ifdef ROOT_ELEMENT_FLAG | ||
|
|
||
| ROOT_ELEMENT_FLAG(0, AllowInputAssemblerInputLayout, "The app is opting in to using the Input Assembler") |
There was a problem hiding this comment.
Where do these descriptions of the flags show up? If they don't show up anywhere then it'd be great to remove them so I don't have to review them!
There was a problem hiding this comment.
It was added as a previous suggestion from Finn #122396 (comment)
There was a problem hiding this comment.
You don't have to do everything everyone suggests (my suggestions included).
I don't understand the point of adding these descriptions if they're not used for anything.
There was a problem hiding this comment.
Whoops, my original comment wasn't very clear then. I had intended to ask for a description of the enum as a comment, similar to the other ones. Not to add a description field for each of the flags
| @@ -72,6 +73,21 @@ struct ShaderHash { | |||
| std::vector<llvm::yaml::Hex8> Digest; | |||
| }; | |||
|
|
|||
| #define ROOT_ELEMENT_FLAG(Num, Val, Str) bool Val = false; | |||
There was a problem hiding this comment.
I know that this is copying the ShaderFeatureFlags example above, but IMO this would be much clearer if the #define was immediately before the #include. I'd be interested to know if there's a reason for this.
Comment on lines
81
to
88
| uint32_t getEncodedFlags(); | ||
| uint32_t Version; | ||
| uint32_t NumParameters; | ||
| uint32_t RootParametersOffset; | ||
| uint32_t NumStaticSamplers; | ||
| uint32_t StaticSamplersOffset; | ||
|
|
||
| #include "llvm/BinaryFormat/DXContainerConstants.def" |
There was a problem hiding this comment.
Suggested change
| uint32_t getEncodedFlags(); | |
| uint32_t Version; | |
| uint32_t NumParameters; | |
| uint32_t RootParametersOffset; | |
| uint32_t NumStaticSamplers; | |
| uint32_t StaticSamplersOffset; | |
| #include "llvm/BinaryFormat/DXContainerConstants.def" | |
| uint32_t Version; | |
| uint32_t NumParameters; | |
| uint32_t RootParametersOffset; | |
| uint32_t NumStaticSamplers; | |
| uint32_t StaticSamplersOffset; | |
| uint32_t getEncodedFlags(); | |
| #include "llvm/BinaryFormat/DXContainerConstants.def" |
IMO this makes it a bit clearer what's going on and less likely to be confused that there's a getEncodedFlags member variable.
(also, github has messed up the diff of this suggestion and I don't know how to fix it)
bogner
reviewed
Feb 4, 2025
There was a problem hiding this comment.
This is looking fairly close. A couple of comments in addition to Damyan's here.
Also a couple of notes on the tests:
- I see you added a compatibility test that used dxil-dis and then removed it citing feedback on the tests. Was there a conversation that I missed? Is there a reason we don't want to add that test?
- The DXContainerTest also needs some negative test cases - what if the buffer is too small? Do we need to validate that the version is what we expect?
Comment on lines
131
to
133
| RootSignature(StringRef Data) : Data(Data) {} | ||
|
|
||
| Error parse(); |
There was a problem hiding this comment.
It's a bit awkward to have the invariant that a user must call parse immediately after construction for this object to be valid. I would probably opt for making the constructor private and adding a factory method to parse a valid object, so something like:
static Expected<RootSignature> parse(StringRef Data);
Also, do we actually need to store the reference to the string in the root signature object? Would just having the parsed integer values be sufficient?
There was a problem hiding this comment.
I looked at this a bit: this is following an existing pattern used in the same source file. I wonder if it might be better to get this change in following the same conventions used, and then do a follow up PR that just refactors all the places in here that follow that pattern. Then we can decide if it's the right thing to do or not?
Being consistent with existing code has its virtues.
There was a problem hiding this comment.
That sounds reasonable to me.
Comment on lines
837
to
844
| const auto &RS = C.getRootSignature(); | ||
| ASSERT_TRUE(RS.has_value()); | ||
| ASSERT_EQ(RS->getVersion(), 2); | ||
| ASSERT_EQ(RS->getNumParameters(), 0); | ||
| ASSERT_EQ(RS->getRootParametersOffset(), 0); | ||
| ASSERT_EQ(RS->getNumStaticSamplers(), 0); | ||
| ASSERT_EQ(RS->getStaticSamplersOffset(), 0); | ||
| ASSERT_EQ(RS->getFlags(), 0x01); |
There was a problem hiding this comment.
Similar to Damyan's suggestion above, we should really use non-zero values for the fields here to ensure that we're reading things from the right places. As is, if we changed the order of the fields in the parser arbitrarily this test would still pass, which is not great.
There was a problem hiding this comment.
I think the idea here is that there'll be additional tests in the future that add parameters and samplers, and at that point we'll be testing these fields.
I don't think we can set any of these parameters to non-zero (apart from maybe the offsets?) without actually providing the parameters - or having invalid data.
There was a problem hiding this comment.
@damyanp is correct. Most of those values need to be 0, mainly NumParameters and NumStaticSamplers. If those values are non zero, dxc and it's tools will try to parse the respective parameters and Static Samplers. Which will be added in the future.
My plan is to add some more testing for those whenever we support those. I modify the other test to have non zero value instead.
bogner
approved these changes
Feb 5, 2025
llvm-beanz
reviewed
Feb 7, 2025
llvm/lib/Object/DXContainer.cpp
Outdated
Comment on lines
249
to
251
| if (Data.size() < 6 * sizeof(uint32_t)) { | ||
| return parseFailed("Invalid data. Too small."); | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (Data.size() < 6 * sizeof(uint32_t)) { | |
| return parseFailed("Invalid data. Too small."); | |
| } | |
| if (Data.size() < 6 * sizeof(uint32_t)) | |
| return parseFailed("Invalid data. Too small."); |
There was a problem hiding this comment.
Missed that, thanks
llvm/lib/Object/DXContainer.cpp
Outdated
|
|
||
| // Root Signature headers expects 6 integers to be present. | ||
| if (Data.size() < 6 * sizeof(uint32_t)) { | ||
| return parseFailed("Invalid data. Too small."); |
There was a problem hiding this comment.
The error here should probably be more descriptive. Imagine having a tool print this error? Without context it doesn't really tell you what went wrong.
Maybe something more like "Invalid root signature, insufficient space for header."
|
|
||
| static Expected<uint32_t> validateVersion(uint32_t Version) { | ||
| if (Version < 1 || Version > 2) | ||
| return llvm::make_error<BinaryStreamError>("Invalid Version"); |
There was a problem hiding this comment.
What version is invalid?
Suggested change
| return llvm::make_error<BinaryStreamError>("Invalid Version"); | |
| return llvm::make_error<BinaryStreamError>("Invalid Root Signature Version"); |
|
|
||
| static Expected<uint32_t> validateRootFlag(uint32_t Flags) { | ||
| if ((Flags & ~0x80000fff) != 0) | ||
| return llvm::make_error<BinaryStreamError>("Invalid flag"); |
There was a problem hiding this comment.
Suggested change
| return llvm::make_error<BinaryStreamError>("Invalid flag"); | |
| return llvm::make_error<BinaryStreamError>("Invalid Root Signature flag"); |
damyanp
approved these changes
Feb 7, 2025
damyanp
approved these changes
Feb 7, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/8488 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/14306 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/4993 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/22315 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2249 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/2207 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
This PR adds: - `RootSignatureFlags` extraction from DXContainer using `obj2yaml` This PR is part of: llvm#121493 --------- Co-authored-by: joaosaffran <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds:
RootSignatureFlagsextraction from DXContainer usingobj2yamlThis PR is part of: #121493