Skip to content

[HLSL][RootSignature] Implement serialized dump of Descriptor Tables #138326

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

Merged
merged 7 commits into from
May 9, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented May 2, 2025

  • defines the dump method for in-memory descriptor table data structs in Frontend/HLSLRootSignature
  • creates unit test infrastructure to support unit tests of the dump methods

Resolves #138189

@llvmbot llvmbot added the HLSL HLSL Language Support label May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-hlsl

Author: Finn Plummer (inbelic)

Changes
  • defines the dump method for in-memory descriptor table data structs in Frontend/HLSLRootSignature
  • creates unit test infrastructure to support unit tests of the dump methods

Resolves #138189


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

5 Files Affected:

  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+7)
  • (modified) llvm/lib/Frontend/HLSL/CMakeLists.txt (+1)
  • (added) llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp (+154)
  • (modified) llvm/unittests/Frontend/CMakeLists.txt (+2)
  • (added) llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp (+111)
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 818caccfe1998..6d3ecd166f461 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -15,6 +15,7 @@
 #define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
 
 #include "llvm/Support/DXILABI.h"
+#include "llvm/Support/raw_ostream.h"
 #include <variant>
 
 namespace llvm {
@@ -52,12 +53,16 @@ enum class RegisterType { BReg, TReg, UReg, SReg };
 struct Register {
   RegisterType ViewType;
   uint32_t Number;
+
+  void dump(raw_ostream &OS) const;
 };
 
 // Models the end of a descriptor table and stores its visibility
 struct DescriptorTable {
   ShaderVisibility Visibility = ShaderVisibility::All;
   uint32_t NumClauses = 0; // The number of clauses in the table
+
+  void dump(raw_ostream &OS) const;
 };
 
 static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
@@ -86,6 +91,8 @@ struct DescriptorTableClause {
       break;
     }
   }
+
+  void dump(raw_ostream &OS) const;
 };
 
 // Models RootElement : DescriptorTable | DescriptorTableClause
diff --git a/llvm/lib/Frontend/HLSL/CMakeLists.txt b/llvm/lib/Frontend/HLSL/CMakeLists.txt
index 07a0c845ceef6..dd987544fe80b 100644
--- a/llvm/lib/Frontend/HLSL/CMakeLists.txt
+++ b/llvm/lib/Frontend/HLSL/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_llvm_component_library(LLVMFrontendHLSL
   CBuffer.cpp
   HLSLResource.cpp
+  HLSLRootSignature.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
new file mode 100644
index 0000000000000..a5f0313138521
--- /dev/null
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -0,0 +1,154 @@
+//===- HLSLRootSignature.cpp - HLSL 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 helpers for working with HLSL Root Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/ADT/bit.h"
+
+namespace llvm {
+namespace hlsl {
+namespace rootsig {
+
+static void dumpRegType(raw_ostream &OS, RegisterType Type) {
+  switch (Type) {
+  case RegisterType::BReg:
+    OS << "b";
+    break;
+  case RegisterType::TReg:
+    OS << "t";
+    break;
+  case RegisterType::UReg:
+    OS << "u";
+    break;
+  case RegisterType::SReg:
+    OS << "s";
+    break;
+  }
+}
+
+void Register::dump(raw_ostream &OS) const {
+  dumpRegType(OS, ViewType);
+  OS << Number;
+}
+
+static void dumpVisibility(raw_ostream &OS, ShaderVisibility Visibility) {
+  switch (Visibility) {
+  case ShaderVisibility::All:
+    OS << "All";
+    break;
+  case ShaderVisibility::Vertex:
+    OS << "Vertex";
+    break;
+  case ShaderVisibility::Hull:
+    OS << "Hull";
+    break;
+  case ShaderVisibility::Domain:
+    OS << "Domain";
+    break;
+  case ShaderVisibility::Geometry:
+    OS << "Geometry";
+    break;
+  case ShaderVisibility::Pixel:
+    OS << "Pixel";
+    break;
+  case ShaderVisibility::Amplification:
+    OS << "Amplification";
+    break;
+  case ShaderVisibility::Mesh:
+    OS << "Mesh";
+    break;
+  }
+}
+
+void DescriptorTable::dump(raw_ostream &OS) const {
+  OS << "DescriptorTable(numClauses = " << NumClauses;
+  OS << ", visibility = ";
+  dumpVisibility(OS, Visibility);
+  OS << ")";
+}
+
+static void dumpClauseType(raw_ostream &OS, ClauseType Type) {
+  switch (Type) {
+  case ClauseType::CBuffer:
+    OS << "CBV";
+    break;
+  case ClauseType::SRV:
+    OS << "SRV";
+    break;
+  case ClauseType::UAV:
+    OS << "UAV";
+    break;
+  case ClauseType::Sampler:
+    OS << "Sampler";
+    break;
+  }
+}
+
+static void dumpDescriptorRangeFlag(raw_ostream &OS, unsigned Bit) {
+  switch (static_cast<DescriptorRangeFlags>(Bit)) {
+  case DescriptorRangeFlags::DescriptorsVolatile:
+    OS << "DescriptorsVolatile";
+    break;
+  case DescriptorRangeFlags::DataVolatile:
+    OS << "DataVolatile";
+    break;
+  case DescriptorRangeFlags::DataStaticWhileSetAtExecute:
+    OS << "DataStaticWhileSetAtExecute";
+    break;
+  case DescriptorRangeFlags::DataStatic:
+    OS << "DataStatic";
+    break;
+  case DescriptorRangeFlags::DescriptorsStaticKeepingBufferBoundsChecks:
+    OS << "DescriptorsStaticKeepingBufferBoundsChecks";
+    break;
+  default:
+    OS << "invalid: " << Bit;
+    break;
+  }
+}
+
+static void dumpDescriptorRangeFlags(raw_ostream &OS,
+                                     DescriptorRangeFlags Flags) {
+  bool FlagSet = false;
+  unsigned Remaining = llvm::to_underlying(Flags);
+  while (Remaining) {
+    unsigned Bit = 1u << llvm::countr_zero(Remaining);
+    if (Remaining & Bit) {
+      if (FlagSet)
+        OS << " | ";
+      dumpDescriptorRangeFlag(OS, Bit);
+      FlagSet = true;
+    }
+    Remaining &= ~Bit;
+  }
+  if (!FlagSet)
+    OS << "None";
+}
+
+void DescriptorTableClause::dump(raw_ostream &OS) const {
+  dumpClauseType(OS, Type);
+  OS << "(";
+  Reg.dump(OS);
+  OS << ", numDescriptors = " << NumDescriptors;
+  OS << ", space = " << Space;
+  OS << ", offset = ";
+  if (Offset == DescriptorTableOffsetAppend)
+    OS << "DescriptorTableOffsetAppend";
+  else
+    OS << Offset;
+  OS << ", flags = ";
+  dumpDescriptorRangeFlags(OS, Flags);
+  OS << ")";
+}
+
+} // namespace rootsig
+} // namespace hlsl
+} // namespace llvm
diff --git a/llvm/unittests/Frontend/CMakeLists.txt b/llvm/unittests/Frontend/CMakeLists.txt
index 85e113816e3bc..2119642769e3d 100644
--- a/llvm/unittests/Frontend/CMakeLists.txt
+++ b/llvm/unittests/Frontend/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Analysis
   Core
+  FrontendHLSL
   FrontendOpenACC
   FrontendOpenMP
   Passes
@@ -10,6 +11,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_llvm_unittest(LLVMFrontendTests
+  HLSLRootSignatureDumpTest.cpp
   OpenACCTest.cpp
   OpenMPContextTest.cpp
   OpenMPIRBuilderTest.cpp
diff --git a/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
new file mode 100644
index 0000000000000..ba1fbfd1f8708
--- /dev/null
+++ b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
@@ -0,0 +1,111 @@
+//===-------- HLSLRootSignatureDumpTest.cpp - RootSignature dump tests ----===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "gtest/gtest.h"
+
+using namespace llvm::hlsl::rootsig;
+
+namespace {
+
+TEST(HLSLRootSignatureTest, DescriptorCBVClauseDump) {
+  DescriptorTableClause Clause;
+  Clause.Type = ClauseType::CBuffer;
+  Clause.Reg = {RegisterType::BReg, 0};
+  Clause.setDefaultFlags();
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  Clause.dump(OS);
+  OS.flush();
+
+  std::string Expected = "CBV(b0, numDescriptors = 1, space = 0, "
+                         "offset = DescriptorTableOffsetAppend, "
+                         "flags = DataStaticWhileSetAtExecute)";
+  EXPECT_EQ(Out, Expected);
+}
+
+TEST(HLSLRootSignatureTest, DescriptorSRVClauseDump) {
+  DescriptorTableClause Clause;
+  Clause.Type = ClauseType::SRV;
+  Clause.Reg = {RegisterType::TReg, 0};
+  Clause.NumDescriptors = 2;
+  Clause.Space = 42;
+  Clause.Offset = 3;
+  Clause.Flags = DescriptorRangeFlags::None;
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  Clause.dump(OS);
+  OS.flush();
+
+  std::string Expected =
+      "SRV(t0, numDescriptors = 2, space = 42, offset = 3, flags = None)";
+  EXPECT_EQ(Out, Expected);
+}
+
+TEST(HLSLRootSignatureTest, DescriptorUAVClauseDump) {
+  DescriptorTableClause Clause;
+  Clause.Type = ClauseType::UAV;
+  Clause.Reg = {RegisterType::UReg, 92374};
+  Clause.NumDescriptors = 3298;
+  Clause.Space = 932847;
+  Clause.Offset = 1;
+  Clause.Flags = DescriptorRangeFlags::ValidFlags;
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  Clause.dump(OS);
+  OS.flush();
+
+  std::string Expected =
+      "UAV(u92374, numDescriptors = 3298, space = 932847, offset = 1, flags = "
+      "DescriptorsVolatile | "
+      "DataVolatile | "
+      "DataStaticWhileSetAtExecute | "
+      "DataStatic | "
+      "DescriptorsStaticKeepingBufferBoundsChecks)";
+  EXPECT_EQ(Out, Expected);
+}
+
+TEST(HLSLRootSignatureTest, DescriptorSamplerClauseDump) {
+  DescriptorTableClause Clause;
+  Clause.Type = ClauseType::Sampler;
+  Clause.Reg = {RegisterType::SReg, 0};
+  Clause.NumDescriptors = 2;
+  Clause.Space = 42;
+  Clause.Offset = DescriptorTableOffsetAppend;
+  Clause.Flags = DescriptorRangeFlags::ValidSamplerFlags;
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  Clause.dump(OS);
+  OS.flush();
+
+  std::string Expected = "Sampler(s0, numDescriptors = 2, space = 42, offset = "
+                         "DescriptorTableOffsetAppend, "
+                         "flags = DescriptorsVolatile)";
+  EXPECT_EQ(Out, Expected);
+}
+
+TEST(HLSLRootSignatureTest, DescriptorTableDump) {
+  DescriptorTable Table;
+  Table.NumClauses = 4;
+  Table.Visibility = ShaderVisibility::Geometry;
+
+  std::string Out;
+  llvm::raw_string_ostream OS(Out);
+  Table.dump(OS);
+  OS.flush();
+
+  std::string Expected =
+      "DescriptorTable(numClauses = 4, visibility = Geometry)";
+  EXPECT_EQ(Out, Expected);
+}
+
+} // namespace

Copy link
Contributor

@joaosaffran joaosaffran left a comment

Choose a reason for hiding this comment

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

nit: Is it possible to have invalid data when serializing ? What is expected in such scenario?

@inbelic
Copy link
Contributor Author

inbelic commented May 8, 2025

The data structures are only constructed through using the RootSignatureParser, so they will have been had basic validation (enums/flags will be valid). I don't think that we should be concerned with handling invalid data-values.

namespace hlsl {
namespace rootsig {

static void dumpRegType(raw_ostream &OS, RegisterType Type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this whole file becomes a bit simpler and cleaner if we define a bunch of operator<<s instead of dump functions. So here we could have

static raw_ostream &operator<<(raw_ostream &OS, const Register &Reg) {
  switch (Reg.Type) {
  case RegisterType::BReg:
    OS << "b";
    break;
  // ...
  }
  OS << Number;
  return OS;

and similarly for the rest of these static dump functions. Then the publicly visible dump() methods can be implemented as OS << "Foo(" << thing1 << "bar = " << thing2 << ")" etc

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

- nicely improves code readability
@inbelic inbelic merged commit 74ed334 into llvm:main May 9, 2025
10 of 12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 10, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-darwin running on doug-worker-3 while building llvm at step 2 "checkout".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/10148

Here is the relevant piece of the build log for the reference
Step 2 (checkout) failure: update (failure)

@llvm-ci
Copy link
Collaborator

llvm-ci commented May 10, 2025

LLVM Buildbot has detected a new failure on builder clang-ppc64-aix running on aix-ppc64 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/3488

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: timeout-hang.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 13
not env -u FILECHECK_OPTS "/home/llvm/llvm-external-buildbots/workers/env/bin/python3.11" /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical Inputs/timeout-hang/run-nonexistent.txt  --timeout=1 --param external=0 | "/home/llvm/llvm-external-buildbots/workers/env/bin/python3.11" /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/timeout-hang.py 1
# executed command: not env -u FILECHECK_OPTS /home/llvm/llvm-external-buildbots/workers/env/bin/python3.11 /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical Inputs/timeout-hang/run-nonexistent.txt --timeout=1 --param external=0
# .---command stderr------------
# | lit.py: /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 1 seconds was requested on the command line. Forcing timeout to be 1 seconds.
# `-----------------------------
# executed command: /home/llvm/llvm-external-buildbots/workers/env/bin/python3.11 /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/build/utils/lit/tests/timeout-hang.py 1
# .---command stdout------------
# | Testing took as long or longer than timeout
# `-----------------------------
# error: command failed with exit status: 1

--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented May 10, 2025

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/2888

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/90/95' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-12196-90-95.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=95 GTEST_SHARD_INDEX=90 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************


@inbelic inbelic deleted the inbelic/rs-dump-descriptor-table branch May 16, 2025 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Implement serialization of Descriptor Tables
6 participants