Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Doc][HLSL] Add documentation for root signature. #83933

Closed
wants to merge 21 commits into from

Conversation

python3kgae
Copy link
Contributor

This patch adds documentation for the root signature in HLSL.

For issue #55116

This patch adds documentation for the root signature in HLSL.
For issue llvm#55116
@python3kgae python3kgae added the HLSL HLSL Language Support label Mar 5, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 5, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Xiang Li (python3kgae)

Changes

This patch adds documentation for the root signature in HLSL.

For issue #55116


Full diff: https://github.com/llvm/llvm-project/pull/83933.diff

1 Files Affected:

  • (added) clang/docs/HLSL/RootSignature.rst (+210)
diff --git a/clang/docs/HLSL/RootSignature.rst b/clang/docs/HLSL/RootSignature.rst
new file mode 100644
index 00000000000000..c2f57340dc2730
--- /dev/null
+++ b/clang/docs/HLSL/RootSignature.rst
@@ -0,0 +1,210 @@
+====================
+HLSL Root Signatures
+====================
+
+.. contents::
+   :local:
+
+Usage
+=====
+
+In HLSL, the `root signature
+<https://learn.microsoft.com/en-us/windows/win32/direct3d12/root-signatures>`_ 
+defines what types of resources are bound to the graphics pipeline. 
+
+A root signature can be specified in HLSL as a `string
+<https://learn.microsoft.com/en-us/windows/win32/direct3d12/specifying-root-signatures-in-hlsl#an-example-hlsl-root-signature>`_. 
+The string contains a collection of comma-separated clauses that describe root 
+signature constituent components. 
+
+There are two mechanisms to compile an HLSL root signature. First, it is 
+possible to attach a root signature string to a particular shader via the 
+RootSignature attribute (in the following example, using the MyRS1 entry 
+point):
+
+.. code-block:: c++
+
+    [RootSignature(MyRS1)]
+    float4 main(float4 coord : COORD) : SV_Target
+    {
+
+    }
+
+The compiler will create and verify the root signature blob for the shader and 
+embed it alongside the shader byte code into the shader blob. 
+
+The other mechanism is to create a standalone root signature blob, perhaps to 
+reuse it with a large set of shaders, saving space. The name of the define 
+string is specified via the usual -E argument. For example:
+
+.. code-block:: c++
+  dxc.exe -T rootsig_1_1 MyRS1.hlsl -E MyRS1 -Fo MyRS1.fxo
+
+Note that the root signature string define can also be passed on the command 
+line, e.g, -D MyRS1=”…”.
+
+Root Signature Grammar
+======================
+
+.. code-block:: c++
+
+    RootSignature : (RootElement(,RootElement)?)?
+
+    RootElement : RootFlags | RootConstants | RootCBV | RootSRV | RootUAV | DescriptorTable | StaticSampler
+
+    RootFlags : 'RootFlags' '(' (RootFlag(|RootFlag)?)? ')'
+
+    RootFlag : 'ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT' | 'DENY_VERTEX_SHADER_ROOT_ACCESS'
+
+    RootConstants : 'RootConstants' '(' 'num32BitConstants' '=' NUMBER ',' bReg (',' 'space' '=' NUMBER)? (',' 'visibility' '=' SHADER_VISIBILITY)? ')'
+
+    RootCBV : 'CBV' '(' bReg (',' 'space' '=' NUMBER)? (',' 'visibility' '=' SHADER_VISIBILITY)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    RootSRV : 'SRV' '(' tReg (',' 'space' '=' NUMBER)? (',' 'visibility' '=' SHADER_VISIBILITY)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    RootUAV : 'UAV' '(' uReg (',' 'space' '=' NUMBER)? (',' 'visibility' '=' SHADER_VISIBILITY)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    DescriptorTable : 'DescriptorTable' '(' (DTClause(|DTClause)?)? (',' 'visibility' '=' SHADER_VISIBILITY)? ')'
+
+    DTClause : CBV | SRV | UAV | Sampler
+
+    CBV : 'CBV' '(' bReg (',' 'numDescriptors' '=' NUMBER)? (',' 'space' '=' NUMBER)? (',' 'offset' '=' DESCRIPTOR_RANGE_OFFSET)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    SRV : 'SRV' '(' tReg (',' 'numDescriptors' '=' NUMBER)? (',' 'space' '=' NUMBER)? (',' 'offset' '=' DESCRIPTOR_RANGE_OFFSET)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    UAV : 'UAV' '(' uReg (',' 'numDescriptors' '=' NUMBER)? (',' 'space' '=' NUMBER)? (',' 'offset' '=' DESCRIPTOR_RANGE_OFFSET)? (',' 'flags' '=' DATA_FLAGS)? ')'
+
+    Sampler : 'Sampler' '(' sReg (',' 'numDescriptors' '=' NUMBER)? (',' 'space' '=' NUMBER)? (',' 'offset' '=' DESCRIPTOR_RANGE_OFFSET)? (',' 'flags' '=' NUMBER)? ')'
+
+
+    SHADER_VISIBILITY : 'SHADER_VISIBILITY_ALL' | 'SHADER_VISIBILITY_VERTEX' | 'SHADER_VISIBILITY_HULL' | 'SHADER_VISIBILITY_DOMAIN' | 'SHADER_VISIBILITY_GEOMETRY' | 'SHADER_VISIBILITY_PIXEL' | 'SHADER_VISIBILITY_AMPLIFICATION' | 'SHADER_VISIBILITY_MESH'
+
+    DATA_FLAGS : 'DATA_STATIC_WHILE_SET_AT_EXECUTE' | 'DATA_VOLATILE'
+
+    DESCRIPTOR_RANGE_OFFSET : 'DESCRIPTOR_RANGE_OFFSET_APPEND' | NUMBER
+
+    StaticSampler : 'StaticSampler' '(' sReg (',' 'filter' '=' FILTER)? (',' 'addressU' '=' TEXTURE_ADDRESS)? (',' 'addressV' '=' TEXTURE_ADDRESS)? (',' 'addressW' '=' TEXTURE_ADDRESS)? (',' 'mipLODBias' '=' NUMBER)? (',' 'maxAnisotropy' '=' NUMBER)? (',' 'comparisonFunc' '=' COMPARISON_FUNC)? (',' 'borderColor' '=' STATIC_BORDER_COLOR)? (',' 'minLOD' '=' NUMBER)? (',' 'maxLOD' '=' NUMBER)? (',' 'space' '=' NUMBER)? (',' 'visibility' '=' SHADER_VISIBILITY)? ')'
+
+    bReg : 'b' NUMBER 
+
+    tReg : 't' NUMBER 
+
+    uReg : 'u' NUMBER 
+
+    sReg : 's' NUMBER 
+
+    FILTER : 'FILTER_MIN_MAG_MIP_POINT' | 'FILTER_MIN_MAG_POINT_MIP_LINEAR' | 'FILTER_MIN_POINT_MAG_LINEAR_MIP_POINT' | 'FILTER_MIN_POINT_MAG_MIP_LINEAR' | 'FILTER_MIN_LINEAR_MAG_MIP_POINT' | 'FILTER_MIN_LINEAR_MAG_POINT_MIP_LINEAR' | 'FILTER_MIN_MAG_LINEAR_MIP_POINT' | 'FILTER_MIN_MAG_MIP_LINEAR' | 'FILTER_ANISOTROPIC' | 'FILTER_COMPARISON_MIN_MAG_MIP_POINT' | 'FILTER_COMPARISON_MIN_MAG_POINT_MIP_LINEAR' | 'FILTER_COMPARISON_MIN_POINT_MAG_LINEAR_MIP_POINT' | 'FILTER_COMPARISON_MIN_POINT_MAG_MIP_LINEAR' | 'FILTER_COMPARISON_MIN_LINEAR_MAG_MIP_POINT' | 'FILTER_COMPARISON_MIN_LINEAR_MAG_POINT_MIP_LINEAR' | 'FILTER_COMPARISON_MIN_MAG_LINEAR_MIP_POINT' | 'FILTER_COMPARISON_MIN_MAG_MIP_LINEAR' | 'FILTER_COMPARISON_ANISOTROPIC' | 'FILTER_MINIMUM_MIN_MAG_MIP_POINT' | 'FILTER_MINIMUM_MIN_MAG_POINT_MIP_LINEAR' | 'FILTER_MINIMUM_MIN_POINT_MAG_LINEAR_MIP_POINT' | 'FILTER_MINIMUM_MIN_POINT_MAG_MIP_LINEAR' | 'FILTER_MINIMUM_MIN_LINEAR_MAG_MIP_POINT' | 'FILTER_MINIMUM_MIN_LINEAR_MAG_POINT_MIP_LINEAR' | 'FILTER_MINIMUM_MIN_MAG_LINEAR_MIP_POINT' | 'FILTER_MINIMUM_MIN_MAG_MIP_LINEAR' | 'FILTER_MINIMUM_ANISOTROPIC' | 'FILTER_MAXIMUM_MIN_MAG_MIP_POINT' | 'FILTER_MAXIMUM_MIN_MAG_POINT_MIP_LINEAR' | 'FILTER_MAXIMUM_MIN_POINT_MAG_LINEAR_MIP_POINT' | 'FILTER_MAXIMUM_MIN_POINT_MAG_MIP_LINEAR' | 'FILTER_MAXIMUM_MIN_LINEAR_MAG_MIP_POINT' | 'FILTER_MAXIMUM_MIN_LINEAR_MAG_POINT_MIP_LINEAR' | 'FILTER_MAXIMUM_MIN_MAG_LINEAR_MIP_POINT' | 'FILTER_MAXIMUM_MIN_MAG_MIP_LINEAR' | 'FILTER_MAXIMUM_ANISOTROPIC'
+
+    TEXTURE_ADDRESS : 'TEXTURE_ADDRESS_WRAP' | 'TEXTURE_ADDRESS_MIRROR' | 'TEXTURE_ADDRESS_CLAMP' | 'TEXTURE_ADDRESS_BORDER' | 'TEXTURE_ADDRESS_MIRROR_ONCE'
+
+    COMPARISON_FUNC : 'COMPARISON_NEVER' | 'COMPARISON_LESS' | 'COMPARISON_EQUAL' | 'COMPARISON_LESS_EQUAL' | 'COMPARISON_GREATER' | 'COMPARISON_NOT_EQUAL' | 'COMPARISON_GREATER_EQUAL' | 'COMPARISON_ALWAYS'
+
+    STATIC_BORDER_COLOR : 'STATIC_BORDER_COLOR_TRANSPARENT_BLACK' | 'STATIC_BORDER_COLOR_OPAQUE_BLACK' | 'STATIC_BORDER_COLOR_OPAQUE_WHITE'
+
+
+Serialized format
+======================
+The root signature string is parsed and serialized into a binary format. The
+binary format is a sequence of bytes that can be used to create a root signature
+object in the Direct3D 12 API. The binary format is defined by the
+`D3D12_ROOT_SIGNATURE_DESC (for rootsig_1_0)
+<https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_root_signature_desc>`_
+or `D3D12_ROOT_SIGNATURE_DESC1 (for rootsig_1_1)
+<https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ns-d3d12-d3d12_root_signature_desc1>`_ 
+structure in the Direct3D 12 API.
+
+
+
+Implementation Details
+======================
+
+The root signature string will be parsed in the HLSL frontend. 
+The parsing 
+will happened when build HLSLRootSignatureAttr or when build standalone root 
+signature blob. 
+
+The root signature parsing will generate a VersionedRootSignatureDesc object 
+that represents the root signature string. 
+VersionedRootSignatureDesc is a struct that contains a RootSignatureVersion 
+and a RootSignatureDesc.
+
+.. code-block:: c++
+    struct DescriptorRange {
+    DescriptorRangeType RangeType;
+    uint32_t NumDescriptors = 1;
+    uint32_t BaseShaderRegister;
+    uint32_t RegisterSpace = 0;
+    DescriptorRangeFlags Flags = DescriptorRangeFlags::None;
+    uint32_t OffsetInDescriptorsFromTableStart = DescriptorRangeOffsetAppend;
+    };
+
+    struct RootDescriptorTable {
+    std::vector<DescriptorRange> DescriptorRanges;
+    };
+    struct RootConstants {
+    uint32_t ShaderRegister;
+    uint32_t RegisterSpace = 0;
+    uint32_t Num32BitValues;
+    };
+
+    struct RootDescriptor {
+    uint32_t ShaderRegister;
+    uint32_t RegisterSpace = 0;
+    RootDescriptorFlags Flags = RootDescriptorFlags::None;
+    };
+    struct RootParameter {
+    RootParameterType ParameterType;
+    std::variant<RootDescriptorTable, RootConstants, RootDescriptor>
+        Parameter;
+    ShaderVisibility ShaderVisibility = ShaderVisibility::All;
+    };
+
+    struct StaticSamplerDesc {
+    Filter Filter = Filter::ANISOTROPIC;
+    TextureAddressMode AddressU = TextureAddressMode::Wrap;
+    TextureAddressMode AddressV = TextureAddressMode::Wrap;
+    TextureAddressMode AddressW = TextureAddressMode::Wrap;
+    float MipLODBias = 0.f;
+    uint32_t MaxAnisotropy = 16;
+    ComparisonFunc ComparisonFunc = ComparisonFunc::LessEqual;
+    StaticBorderColor BorderColor = StaticBorderColor::OpaqueWhite;
+    float MinLOD = 0.f;
+    float MaxLOD = MaxLOD;
+    uint32_t ShaderRegister;
+    uint32_t RegisterSpace = 0;
+    ShaderVisibility ShaderVisibility = ShaderVisibility::All;
+    };
+
+    struct RootSignatureDesc {
+    std::vector<RootParameter> Parameters;
+    std::vector<StaticSamplerDesc> StaticSamplers;
+    RootSignatureFlags Flags;
+    };
+
+    struct VersionedRootSignatureDesc {
+    RootSignatureVersion Version;
+    RootSignatureDesc Desc;
+    };
+
+Things like DescriptorRangeType and RootDescriptorFlags will be enums.
+
+After parsing, the VersionedRootSignatureDesc will be translated into a 
+constant global variable in the clang AST and save to the 
+HLSLRootSignatureAttr. 
+
+For case compile to a standalone root signature blob, the global variable will 
+be saved in the ASTContext.
+
+The global variable in AST will have a struct type that represents the root signature
+layout and a initializer that contains the values like space and 
+numDescriptors of the root signature.
+
+In clang code generation, the global variable in AST will be translated into a global 
+variable with cosntant initializer in LLVM IR. 
+
+CGHLSLRuntime will generate metadata to link the global variable as root 
+signature for given entry function or just nullptr for the standalone root 
+signature blob case. 
+
+In LLVM DirectX backend, the global variable will be serialized and save into the root
+signature part of dx container when emit DXIL.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Sema-level diagnostics?

How do we expect to handle and surface parsing errors in the root signature?

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
resource in the root signature. It will bind to the entry function in the AST.

For case compile to a standalone root signature blob, the HLSLRootSignatureAttr
will be bind to the TU.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious for @tex3d's thoughts about an alternate approach here. I think there is a way we can change how the rootsig_1_x profiles are implemented to be less hacky but still maintain source compatibility with existing sources.

My thought is that we always attach the HLSLRootSignatureAttr to the entry function, but in the rootsig profiles we allow the entry function to be a declaration instead of a full definition. Existing rootsig shaders take a form like:

// RUN:%dxc -T rootsig_1_1 %s -rootsig-define main
// RUN:%dxc -T rootsig_1_1 %s -E main

#define main "CBV(b0), RootFlags(LOCAL_ROOT_SIGNATURE)"

When targeting the rootsig profile we could implicitly add this to the translation unit:

[RootSignature(main)]
#undef main
void main();

That would give us a declaration for an entry function, and a root signature attribute applied to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea.
The fake entry needs to be added to the end and before preprocess expand the macro.
It doesn't need to be a declare, a definition should be OK.
Since we know the target profile, and just generate the global variable with section 'RTS0' and remove the empty function before MC ObjectWriter.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran into some problems when building clang-docs-html to look at this, so here are some notes on fixing those issues. I'll do another pass on the content next.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
@damyanp damyanp self-requested a review March 27, 2024 22:17
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
protected:
std::string RootSignatureStr;
VersionedRootSignatureDesc RootSignature;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can simply attach this extra information to an InheritableAttr like this. At least, no other attributes do this and it feels like a break in abstraction. There isn't really any prior art for embedding an entire mini-language to define a structure in an attribute as far as I know, so we need to be careful to do this right.

Looking at the clang internals manual and how Parser::ParseAvailabilityAttribute handles custom parsing, I think that it's possible to do something along these lines but it would be very invasive. We should get input from some of the clang folks that are familiar with attribute handling (like @erichkeane perhaps) on whether there's a simpler or better way to go about this. It's possible that attributes are a bad fit here and that we need to find a different way to make this information available to Sema and clang's codegen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asked about put data into AST in discourse.
Here's the link.
https://discourse.llvm.org/t/question-about-storing-data-in-ast-node/78152

Erich said the common approach is only save the string. If the cost of parsing is big, it is OK to save the information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cost of parsing big here though? These seem quite cheap to parse and are not typically going to be very big. It seems like keeping the string throughout the compilation would be the simplest solution and re-parsing it the few times as needed is not a big deal. The main downside is how to layer the paring lib so that it is available everywhere it is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the cost of parsing will be big.

Another question I'm not sure is about other languages which target DirectX backend.
Is it better to build the root signature string for the llvm IR or generate a llvm::ConstantStruct for a frontend which not have the root signature string in the language?
My feeling is string will be easier to build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot one issue Chris mentioned before.
It might be hard to share the parser between clang and llvm for the diagnostic lives is in clang which llvm doesn't have access.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
@bogner
Copy link
Contributor

bogner commented Mar 28, 2024

I think it would be helpful to add a section between the usage part and the grammar that discusses where and why in the compiler we need access to the parsed root signature (ie, in Sema for diagnostics, in the backend to verify resource usage matches, and also in the backend to lower to the serialized form). This would make it a little clearer why we need the various different representations since it will describe what we'll use them for.

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be useful to have a section in this document that briefly describes what is done at each phase of the compiler and what information is passed through to the next.

For example:
ClangSema parses the attribute and generates . ClangCodeGen reads and generates . in the DirectX backend transforms/validates yo . LLVMMC emits to .

Also we need to keep in mind that some of these structures need to be available in the LLVM Binary Format and Object libraries so that the object parser can read root signatures. We should think about what bits of code need to live where to maintain the correct layers in the compiler architecture.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
@dmpots
Copy link
Contributor

dmpots commented Mar 29, 2024

I did not see any discussion of local root signatures here. It is probably good to consider those as well when thinking about this design. There is alternate syntax for specifying both local root signatues and global root signatures starting in SM 6.3

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object#globalrootsignature

GlobalRootSignature MyGlobalRootSignature =
{
    "DescriptorTable(UAV(u0)),"                     // Output texture
    "SRV(t0),"                                      // Acceleration structure
    "CBV(b0),"                                      // Scene constants
    "DescriptorTable(SRV(t1, numDescriptors = 2))"  // Static index and vertex buffers.
};

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object#localrootsignature

LocalRootSignature MyLocalRootSignature = 
{
    "RootConstants(num32BitConstants = 4, b1)"  // Cube constants 
};

@python3kgae
Copy link
Contributor Author

I did not see any discussion of local root signatures here. It is probably good to consider those as well when thinking about this design. There is alternate syntax for specifying both local root signatues and global root signatures starting in SM 6.3

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object#globalrootsignature

GlobalRootSignature MyGlobalRootSignature =
{
    "DescriptorTable(UAV(u0)),"                     // Output texture
    "SRV(t0),"                                      // Acceleration structure
    "CBV(b0),"                                      // Scene constants
    "DescriptorTable(SRV(t1, numDescriptors = 2))"  // Static index and vertex buffers.
};

https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-state-object#localrootsignature

LocalRootSignature MyLocalRootSignature = 
{
    "RootConstants(num32BitConstants = 4, b1)"  // Cube constants 
};

Added local/global root signature to the doc.

clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
clang/docs/HLSL/RootSignature.rst Outdated Show resolved Hide resolved
Co-authored-by: Damyan Pepper <damyanp@microsoft.com>
Copy link
Contributor

@damyanp damyanp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some open comments that don't seem like they've been resolved yet (along with a couple of typos I've pointed out).

With 85 comments this PR is become quite hard to follow. I wonder if we're at a point now where it's either good enough to go in and have issues filed against it, or if we need to abandon this one and start a fresh one?

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to break this PR up into different parts going to different places.

Documenting the RootSignature grammar and usage in the Clang docs makes sense.

I think documentation of the RootSignature binary format probably should live with the DirectX backend docs, since the code to generate the binary encoding should live in LLVM.

The last part of the document here about the implementation I think we should remove from this PR and bring that discussion over to https://github.com/microsoft/hlsl-specs. There are a lot of parts of the design that I'm not convinced are correct or lack appropriate detail for me to understand.

To @damyanp's point I think the discussion thread here has gotten long enough to be unwieldy. Rather than merging an incomplete document to Clang's docs, I think it would be better for us to iterate on the design outside the LLVM source tree and return to LLVM with a design we have a higher level of confidence in.

@python3kgae can you strip PR down to just the first parts that document Root Signatures and take the implementation discussion over to hlsl-specs?

@python3kgae
Copy link
Contributor Author

I think we need to break this PR up into different parts going to different places.

Documenting the RootSignature grammar and usage in the Clang docs makes sense.

I think documentation of the RootSignature binary format probably should live with the DirectX backend docs, since the code to generate the binary encoding should live in LLVM.

The last part of the document here about the implementation I think we should remove from this PR and bring that discussion over to https://github.com/microsoft/hlsl-specs. There are a lot of parts of the design that I'm not convinced are correct or lack appropriate detail for me to understand.

To @damyanp's point I think the discussion thread here has gotten long enough to be unwieldy. Rather than merging an incomplete document to Clang's docs, I think it would be better for us to iterate on the design outside the LLVM source tree and return to LLVM with a design we have a higher level of confidence in.

@python3kgae can you strip PR down to just the first parts that document Root Signatures and take the implementation discussion over to hlsl-specs?

Created microsoft/hlsl-specs#193

@python3kgae python3kgae closed this Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants