Skip to content

[NFC][HLSL][RootSignature] Use operator<< overload instead of dump method #141127

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 1 commit into from
May 23, 2025

Conversation

inbelic
Copy link
Contributor

@inbelic inbelic commented May 22, 2025

  • we will need to provide a way to dump RootFlags for serialization and by using operator overloads we can maintain a consistent interface

This is an NFC to allow for #138192 to be more straightforwardly implemented.

…method

- we will need to provide a way to dump `RootFlags` for serialization
and by using operator overloads we can maintain a consistent interface
@llvmbot llvmbot added the HLSL HLSL Language Support label May 22, 2025
@llvmbot
Copy link
Member

llvmbot commented May 22, 2025

@llvm/pr-subscribers-hlsl

Author: Finn Plummer (inbelic)

Changes
  • we will need to provide a way to dump RootFlags for serialization and by using operator overloads we can maintain a consistent interface

This is an NFC to allow for #138192 to be more straightforwardly implemented.


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

3 Files Affected:

  • (modified) llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h (+4-4)
  • (modified) llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp (+19-14)
  • (modified) llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp (+5-5)
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 7829b842fa2be..7e7eeec0deb52 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -97,10 +97,10 @@ struct DescriptorTable {
   // Denotes that the previous NumClauses in the RootElement array
   // are the clauses in the table.
   uint32_t NumClauses = 0;
-
-  void dump(raw_ostream &OS) const;
 };
 
+raw_ostream &operator<<(raw_ostream &OS, const DescriptorTable &Table);
+
 static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
 static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
 // Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
@@ -127,10 +127,10 @@ struct DescriptorTableClause {
       break;
     }
   }
-
-  void dump(raw_ostream &OS) const;
 };
 
+raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause);
+
 /// Models RootElement : RootFlags | RootConstants | RootDescriptor
 ///  | DescriptorTable | DescriptorTableClause
 ///
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index abf076944b273..ec0d130a6767c 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -71,11 +71,6 @@ static raw_ostream &operator<<(raw_ostream &OS,
   return OS;
 }
 
-void DescriptorTable::dump(raw_ostream &OS) const {
-  OS << "DescriptorTable(numClauses = " << NumClauses
-     << ", visibility = " << Visibility << ")";
-}
-
 static raw_ostream &operator<<(raw_ostream &OS, const ClauseType &Type) {
   switch (Type) {
   case ClauseType::CBuffer:
@@ -137,14 +132,24 @@ static raw_ostream &operator<<(raw_ostream &OS,
   return OS;
 }
 
-void DescriptorTableClause::dump(raw_ostream &OS) const {
-  OS << Type << "(" << Reg << ", numDescriptors = " << NumDescriptors
-     << ", space = " << Space << ", offset = ";
-  if (Offset == DescriptorTableOffsetAppend)
+raw_ostream &operator<<(raw_ostream &OS, const DescriptorTable &Table) {
+  OS << "DescriptorTable(numClauses = " << Table.NumClauses
+     << ", visibility = " << Table.Visibility << ")";
+
+  return OS;
+}
+
+raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause) {
+  OS << Clause.Type << "(" << Clause.Reg
+     << ", numDescriptors = " << Clause.NumDescriptors
+     << ", space = " << Clause.Space << ", offset = ";
+  if (Clause.Offset == DescriptorTableOffsetAppend)
     OS << "DescriptorTableOffsetAppend";
   else
-    OS << Offset;
-  OS << ", flags = " << Flags << ")";
+    OS << Clause.Offset;
+  OS << ", flags = " << Clause.Flags << ")";
+
+  return OS;
 }
 
 void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) {
@@ -154,11 +159,11 @@ void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) {
     if (!First)
       OS << ",";
     OS << " ";
-    First = false;
     if (const auto &Clause = std::get_if<DescriptorTableClause>(&Element))
-      Clause->dump(OS);
+      OS << *Clause;
     if (const auto &Table = std::get_if<DescriptorTable>(&Element))
-      Table->dump(OS);
+      OS << *Table;
+    First = false;
   }
   OS << "}";
 }
diff --git a/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
index ba1fbfd1f8708..3f92fa0f05794 100644
--- a/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
+++ b/llvm/unittests/Frontend/HLSLRootSignatureDumpTest.cpp
@@ -21,7 +21,7 @@ TEST(HLSLRootSignatureTest, DescriptorCBVClauseDump) {
 
   std::string Out;
   llvm::raw_string_ostream OS(Out);
-  Clause.dump(OS);
+  OS << Clause;
   OS.flush();
 
   std::string Expected = "CBV(b0, numDescriptors = 1, space = 0, "
@@ -41,7 +41,7 @@ TEST(HLSLRootSignatureTest, DescriptorSRVClauseDump) {
 
   std::string Out;
   llvm::raw_string_ostream OS(Out);
-  Clause.dump(OS);
+  OS << Clause;
   OS.flush();
 
   std::string Expected =
@@ -60,7 +60,7 @@ TEST(HLSLRootSignatureTest, DescriptorUAVClauseDump) {
 
   std::string Out;
   llvm::raw_string_ostream OS(Out);
-  Clause.dump(OS);
+  OS << Clause;
   OS.flush();
 
   std::string Expected =
@@ -84,7 +84,7 @@ TEST(HLSLRootSignatureTest, DescriptorSamplerClauseDump) {
 
   std::string Out;
   llvm::raw_string_ostream OS(Out);
-  Clause.dump(OS);
+  OS << Clause;
   OS.flush();
 
   std::string Expected = "Sampler(s0, numDescriptors = 2, space = 42, offset = "
@@ -100,7 +100,7 @@ TEST(HLSLRootSignatureTest, DescriptorTableDump) {
 
   std::string Out;
   llvm::raw_string_ostream OS(Out);
-  Table.dump(OS);
+  OS << Table;
   OS.flush();
 
   std::string Expected =

@inbelic inbelic merged commit 5d76555 into llvm:main May 23, 2025
12 of 14 checks passed
@inbelic inbelic deleted the inbelic/nfc-rs-use-operator branch June 2, 2025 20:33
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…method (llvm#141127)

- we will need to provide a way to dump `RootFlags` for serialization
and by using operator overloads we can maintain a consistent interface

This is an NFC to allow for
llvm#138192 to be more
straightforwardly implemented.
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.

4 participants