[DirectX] Move resource logic into PrettyPrinter and TranslateMetadata. NFC

[bogner]

Move the module level logic for resources into the pretty printer and translate
metadata passes rather than embedding them in the DXILResource helper. This
will make it easier to migrate towards the target extension type based approach
to resources.

[llvmbot]

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

Move the module level logic for resources into the pretty printer and translate
metadata passes rather than embedding them in the DXILResource helper. This
will make it easier to migrate towards the target extension type based approach
to resources.

---

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

4 Files Affected:

• (modified) llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp (+16-2)
• (modified) llvm/lib/Target/DirectX/DXILResource.cpp (+8-31)
• (modified) llvm/lib/Target/DirectX/DXILResource.h (+7-2)
• (modified) llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp (+25-1)

diff --git a/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp b/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
index 7d2abb7078b8a..c57631cc4c8b6 100644
--- a/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
+++ b/llvm/lib/Target/DirectX/DXILPrettyPrinter.cpp
@@ -12,13 +12,27 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 static void prettyPrintResources(raw_ostream &OS,
                                  const dxil::Resources &MDResources) {
-  MDResources.print(OS);
+  // Column widths are arbitrary but match the widths DXC uses.
+  OS << ";\n; Resource Bindings:\n;\n";
+  OS << formatv("; {0,-30} {1,10} {2,7} {3,11} {4,7} {5,14} {6,16}\n",
+                "Name", "Type", "Format", "Dim", "ID", "HLSL Bind", "Count");
+  OS << formatv(
+      "; {0,-+30} {1,-+10} {2,-+7} {3,-+11} {4,-+7} {5,-+14} {6,-+16}\n", "",
+      "", "", "", "", "", "");
+
+  if (MDResources.hasCBuffers())
+    MDResources.printCBuffers(OS);
+  if (MDResources.hasUAVs())
+    MDResources.printUAVs(OS);
+
+  OS << ";\n";
 }
 
 PreservedAnalyses DXILPrettyPrinterPass::run(Module &M,
@@ -63,7 +77,7 @@ INITIALIZE_PASS_END(DXILPrettyPrinterLegacy, "dxil-pretty-printer",
 
 bool DXILPrettyPrinterLegacy::runOnModule(Module &M) {
   dxil::Resources &Res = getAnalysis<DXILResourceMDWrapper>().getDXILResource();
-  Res.print(OS);
+  prettyPrintResources(OS, Res);
   return false;
 }
 
diff --git a/llvm/lib/Target/DirectX/DXILResource.cpp b/llvm/lib/Target/DirectX/DXILResource.cpp
index 8e5b9867e6661..f027283b70521 100644
--- a/llvm/lib/Target/DirectX/DXILResource.cpp
+++ b/llvm/lib/Target/DirectX/DXILResource.cpp
@@ -333,37 +333,14 @@ template <typename T> MDNode *ResourceTable<T>::write(Module &M) const {
   return MDNode::get(M.getContext(), MDs);
 }
 
-void Resources::write(Module &M) const {
-  Metadata *ResourceMDs[4] = {nullptr, nullptr, nullptr, nullptr};
-
-  ResourceMDs[1] = UAVs.write(M);
-
-  ResourceMDs[2] = CBuffers.write(M);
-
-  bool HasResource = ResourceMDs[0] != nullptr || ResourceMDs[1] != nullptr ||
-                     ResourceMDs[2] != nullptr || ResourceMDs[3] != nullptr;
-
-  if (HasResource) {
-    NamedMDNode *DXResMD = M.getOrInsertNamedMetadata("dx.resources");
-    DXResMD->addOperand(MDNode::get(M.getContext(), ResourceMDs));
-  }
-
-  NamedMDNode *Entry = M.getNamedMetadata("hlsl.uavs");
-  if (Entry)
-    Entry->eraseFromParent();
+Metadata *Resources::writeUAVs(Module &M) const { return UAVs.write(M); }
+void Resources::printUAVs(raw_ostream &OS) const { UAVs.print(OS); }
+Metadata *Resources::writeCBuffers(Module &M) const {
+  return CBuffers.write(M);
 }
+void Resources::printCBuffers(raw_ostream &OS) const { CBuffers.print(OS); }
 
-void Resources::print(raw_ostream &O) const {
-  O << ";\n"
-    << "; Resource Bindings:\n"
-    << ";\n"
-    << "; Name                                 Type  Format         Dim      "
-       "ID      HLSL Bind  Count\n"
-    << "; ------------------------------ ---------- ------- ----------- "
-       "------- -------------- ------\n";
-
-  CBuffers.print(O);
-  UAVs.print(O);
+void Resources::dump() const {
+  printCBuffers(dbgs());
+  printUAVs(dbgs());
 }
-
-void Resources::dump() const { print(dbgs()); }
diff --git a/llvm/lib/Target/DirectX/DXILResource.h b/llvm/lib/Target/DirectX/DXILResource.h
index 06902fe2b87b0..812729bc4dc57 100644
--- a/llvm/lib/Target/DirectX/DXILResource.h
+++ b/llvm/lib/Target/DirectX/DXILResource.h
@@ -103,6 +103,7 @@ template <typename T> class ResourceTable {
 public:
   ResourceTable(StringRef Name) : MDName(Name) {}
   void collect(Module &M);
+  bool empty() const { return Data.empty(); }
   MDNode *write(Module &M) const;
   void print(raw_ostream &O) const;
 };
@@ -117,8 +118,12 @@ class Resources {
 
 public:
   void collect(Module &M);
-  void write(Module &M) const;
-  void print(raw_ostream &O) const;
+  bool hasUAVs() const { return !UAVs.empty(); }
+  Metadata *writeUAVs(Module &M) const;
+  void printUAVs(raw_ostream &OS) const;
+  bool hasCBuffers() const { return !CBuffers.empty(); }
+  Metadata *writeCBuffers(Module &M) const;
+  void printCBuffers(raw_ostream &OS) const;
   LLVM_DUMP_METHOD void dump() const;
 };
 
diff --git a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
index 55b3d94568426..007af0b46b9f3 100644
--- a/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
+++ b/llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp
@@ -22,6 +22,30 @@
 using namespace llvm;
 using namespace llvm::dxil;
 
+static void emitResourceMetadata(Module &M,
+                                 const dxil::Resources &MDResources) {
+  Metadata *SRVMD = nullptr, *UAVMD = nullptr, *CBufMD = nullptr,
+           *SmpMD = nullptr;
+  bool HasResources = false;
+
+  if (MDResources.hasUAVs()) {
+    UAVMD = MDResources.writeUAVs(M);
+    HasResources = true;
+  }
+
+  if (MDResources.hasCBuffers()) {
+    CBufMD = MDResources.writeCBuffers(M);
+    HasResources = true;
+  }
+
+  if (!HasResources)
+    return;
+
+  NamedMDNode *ResourceMD = M.getOrInsertNamedMetadata("dx.resources");
+  ResourceMD->addOperand(
+      MDNode::get(M.getContext(), {SRVMD, UAVMD, CBufMD, SmpMD}));
+}
+
 static void translateMetadata(Module &M, const dxil::Resources &MDResources,
                               const ComputedShaderFlags &ShaderFlags) {
   dxil::ValidatorVersionMD ValVerMD(M);
@@ -30,7 +54,7 @@ static void translateMetadata(Module &M, const dxil::Resources &MDResources,
   dxil::createShaderModelMD(M);
   dxil::createDXILVersionMD(M);
 
-  MDResources.write(M);
+  emitResourceMetadata(M, MDResources);
 
   dxil::createEntryMD(M, static_cast<uint64_t>(ShaderFlags));
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[hekota]

LGTM!

[damyanp]

What does the "MD" in "MDResources" mean?

[bogner]

"Metadata". This is mostly a temporary thing while both DXILResourceAnalysis and DXILResourceMDAnalysis co-exist. The one with "MD" in the name will eventually be removed.