Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types #114449

coopp · 2024-10-31T18:43:39Z

This commit address Issue #112273 .

This change includes updates to the shader analysis pass and walking the resource metadata to properly set the required shader flags for RWbuffers.

…el version etc.)

llvmbot · 2024-10-31T18:44:13Z

@llvm/pr-subscribers-backend-directx

Author: Cooper Partin (coopp)

Changes

This commit address Issue #112273 .

This change includes updates to the shader analysis pass and walking the resource metadata to properly set the required shader flags for RWbuffers.

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

4 Files Affected:

(modified) llvm/lib/Target/DirectX/DXILShaderFlags.cpp (+32-2)
(modified) llvm/lib/Target/DirectX/DXILShaderFlags.h (+2-2)
(added) llvm/test/CodeGen/DirectX/ShaderFlags/buffers.ll (+23)
(added) llvm/test/CodeGen/DirectX/ShaderFlags/buffers_cs4.ll (+24)

diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 9fa137b4c025e1..e24b8b5e8e594f 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -13,8 +13,11 @@
 
 #include "DXILShaderFlags.h"
 #include "DirectX.h"
+#include "llvm/Analysis/DXILMetadataAnalysis.h"
+#include "llvm/Analysis/DXILResource.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/DXILABI.h"
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace llvm;
@@ -36,8 +39,35 @@ static void updateFlags(ComputedShaderFlags &Flags, const Instruction &I) {
   }
 }
 
-ComputedShaderFlags ComputedShaderFlags::computeFlags(Module &M) {
+static void updateResourceFlags(ComputedShaderFlags &Flags, Module &M,
+                                ModuleAnalysisManager *AM) {
+  if (!AM)
+    return;
+
+  const DXILResourceMap &DRM = AM->getResult<DXILResourceAnalysis>(M);
+  if (DRM.empty())
+    return;
+
+  const dxil::ModuleMetadataInfo &MMDI = AM->getResult<DXILMetadataAnalysis>(M);
+  VersionTuple SM = MMDI.ShaderModelVersion;
+  Triple::EnvironmentType SP = MMDI.ShaderProfile;
+
+  // RWBuffer
+  for (const ResourceInfo &RI : DRM.uavs()) {
+    if (RI.getResourceKind() == ResourceKind::TypedBuffer) {
+      Flags.EnableRawAndStructuredBuffers = true;
+      Flags.ComputeShadersPlusRawAndStructuredBuffers =
+          (SP == Triple::EnvironmentType::Compute && SM.getMajor() == 4);
+      break;
+    }
+  }
+}
+
+ComputedShaderFlags
+ComputedShaderFlags::computeFlags(Module &M, ModuleAnalysisManager *AM) {
   ComputedShaderFlags Flags;
+  updateResourceFlags(Flags, M, AM);
+
   for (const auto &F : M)
     for (const auto &BB : F)
       for (const auto &I : BB)
@@ -67,7 +97,7 @@ AnalysisKey ShaderFlagsAnalysis::Key;
 
 ComputedShaderFlags ShaderFlagsAnalysis::run(Module &M,
                                              ModuleAnalysisManager &AM) {
-  return ComputedShaderFlags::computeFlags(M);
+  return ComputedShaderFlags::computeFlags(M, &AM);
 }
 
 PreservedAnalyses ShaderFlagsAnalysisPrinter::run(Module &M,
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.h b/llvm/lib/Target/DirectX/DXILShaderFlags.h
index 1df7d27de13d3c..86d0a70dcbee10 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.h
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.h
@@ -60,7 +60,7 @@ struct ComputedShaderFlags {
     return FeatureFlags;
   }
 
-  static ComputedShaderFlags computeFlags(Module &M);
+  static ComputedShaderFlags computeFlags(Module &M, ModuleAnalysisManager *AM);
   void print(raw_ostream &OS = dbgs()) const;
   LLVM_DUMP_METHOD void dump() const { print(); }
 };
@@ -102,7 +102,7 @@ class ShaderFlagsAnalysisWrapper : public ModulePass {
   const ComputedShaderFlags &getShaderFlags() { return Flags; }
 
   bool runOnModule(Module &M) override {
-    Flags = ComputedShaderFlags::computeFlags(M);
+    Flags = ComputedShaderFlags::computeFlags(M, nullptr);
     return false;
   }
 
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/buffers.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/buffers.ll
new file mode 100644
index 00000000000000..93346c2d5587db
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/buffers.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+
+target triple = "dxil-pc-shadermodel6.3-compute"
+
+@G = external constant <4 x float>, align 4
+
+define void @test_bufferflags() {
+
+  ; RWBuffer<int> Buf : register(u7, space2)
+  %uav0 = call target("dx.TypedBuffer", i32, 1, 0, 1)
+      @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0t(
+          i32 2, i32 7, i32 1, i32 0, i1 false)
+
+; CHECK: ; Shader Flags Value: 0x00000010
+; CHECK: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ; D3D11_SB_GLOBAL_FLAG_ENABLE_RAW_AND_STRUCTURED_BUFFERS
+; CHECK-NEXT: {{^;$}}
+
+  ret void
+}
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }
diff --git a/llvm/test/CodeGen/DirectX/ShaderFlags/buffers_cs4.ll b/llvm/test/CodeGen/DirectX/ShaderFlags/buffers_cs4.ll
new file mode 100644
index 00000000000000..21b05e4ef59a22
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ShaderFlags/buffers_cs4.ll
@@ -0,0 +1,24 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+
+target triple = "dxil-pc-shadermodel4.0-compute"
+
+@G = external constant <4 x float>, align 4
+
+define void @test_bufferflags() {
+
+  ; RWBuffer<int> Buf : register(u7, space2)
+  %uav0 = call target("dx.TypedBuffer", i32, 1, 0, 1)
+      @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0t(
+          i32 2, i32 7, i32 1, i32 0, i1 false)
+
+; CHECK: ; Shader Flags Value: 0x00020010
+; CHECK: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ;       Raw and Structured buffers
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ; D3D11_SB_GLOBAL_FLAG_ENABLE_RAW_AND_STRUCTURED_BUFFERS
+; CHECK-NEXT: {{^;$}}
+
+  ret void
+}
+
+attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }

coopp · 2024-10-31T18:47:00Z

Hopefully this PR helps validate my approach for setting these flags. I have not figured out how to get strcuturedbuffers to work/be detected yet. I thought I had the correct IR, but they are not being picked up for some reason.

I had this logic in the code which is not in this PR

// StructuredRWBuffer
  for (const ResourceInfo &RI : DRM.srvs()) {
    if (RI.getResourceKind() == ResourceKind::RawBuffer) {
      Flags.EnableRawAndStructuredBuffers = true;
      Flags.ComputeShadersPlusRawAndStructuredBuffers =
          (SP == Triple::EnvironmentType::Compute && SM.getMajor() == 4);
      break;
    }
  }

tex3d · 2024-10-31T18:58:49Z

llvm/lib/Target/DirectX/DXILShaderFlags.cpp

+
+  // RWBuffer
+  for (const ResourceInfo &RI : DRM.uavs()) {
+    if (RI.getResourceKind() == ResourceKind::TypedBuffer) {


This looks wrong. It should only set this if it's a StructuredBuffer or RawBuffer (aka. ByteAddressBuffer), but not if it's a TypedBuffer.

Hmm.. I went for this approach because of how the IR looked. Maybe I missed something here.

; RWBuffer<int> Buf : register(u7, space2) %uav0 = call target("dx.TypedBuffer", i32, 1, 0, 1) @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0t( i32 2, i32 7, i32 1, i32 0, i1 false)

In HLSL: RWBuffer is a typed buffer. You want to set this if one of the HLSL types: RWStructuredBuffer or RWByteAddressBuffer, is used.

I believe I fixed this now.

damyanp · 2024-11-01T19:33:59Z

llvm/lib/Target/DirectX/DXILShaderFlags.cpp

+    if (RI.getResourceKind() == ResourceKind::TypedBuffer) {
+      Flags.EnableRawAndStructuredBuffers = true;
+      Flags.ComputeShadersPlusRawAndStructuredBuffers =
+          (SP == Triple::EnvironmentType::Compute && SM.getMajor() == 4);


SM.getMajor() == 4

This means SM 4.x? In which case this flag will never be set. I'm surprised it isn't >= 4.

Yeah.. I was too! I looked at the DXC source and it does this type of check.. Since I recently learned that there is no way we would even support SM4, we should just not set this flag anyway and remove it.

Probably should get some more opinions on this. If we were to ever support SM 4.0 via DXBC in the future then having this in here would likely be helpful to that effort.

This issue has been resolved in chat. The result is to not set this flag as it will not be used at this time.

…t that went along with it

tex3d

After some further analysis, I don't think we should set this flag in this compiler, unless/until we support ps_4_0/ps_4_1 DXBC targets, where the flag is used in the DXBC instruction stream. This is the only place this flag is used.

tex3d · 2024-11-06T23:51:10Z

llvm/test/CodeGen/DirectX/ShaderFlags/buffer.ll

+; CHECK: ; Shader Flags Value: 0x00000010
+; CHECK: ; Note: shader requires additional functionality:
+; CHECK-NEXT: ; Note: extra DXIL module flags:
+; CHECK-NEXT: ; D3D11_SB_GLOBAL_FLAG_ENABLE_RAW_AND_STRUCTURED_BUFFERS


This isn't one of the optional feature flags used by the runtime. So, it should not have a comment saying "shader requires additional functionality". You can tell which flags should be reported under this statement by looking at the ones set in the ShaderFlags::GetFeatureInfo method in DxilShaderFlags.cpp in DXC.

Instead, this flag mirrors a global flag that was set in the DXBC dcl_globalFlags instruction token: D3D11_SB_GLOBAL_FLAG_ENABLE_RAW_AND_STRUCTURED_BUFFERS. This value this identifier is defined to in the DXBC token stream is not the same value as in the DXIL Shader Flags, so that could be a bit confusing to write that out here as the flag name.

Additionally: It turns out that this flag is only generated for SRV raw/structured buffers on ps_4_0 or ps_4_1. UAV's not allowed for those targets at all.

It only exists in DXIL as the way to preserve this DXBC flag when we were writing the initial version of the DXBC to DXIL converter (DxilConv.dll), which originally would try to preserve the shader model and any flags set in the original DXBC.

So due to these limitations, I believe we shouldn't ever need to set this flag for DXIL in this compiler after all.

I've opened microsoft/DirectXShaderCompiler#7003 to track removal for validation.

coopp · 2024-11-07T19:28:52Z

Closing this pull request as these flags being set are no longer needed or used. Tex opened microsoft/DirectXShaderCompiler#7003 to track addressing what to do with them long term.

Cooper Partin added 8 commits October 25, 2024 13:01

Add simple shader flags tests for structured and raw buffer

cf6b6d8

Fix raw_buffers test

28580c2

cleanup tests

cecb423

Update tests with new IR

98e8969

rename test

607fdc1

Work in progress (contains checks for shader profile type, shader mod…

a3c0af0

…el version etc.)

Add cs4 test and updated logic to handle legacy pass manager

87a63a6

Remove structured buffer because it is not working for some reason

0d820cc

llvmbot added the backend:DirectX label Oct 31, 2024

tex3d reviewed Oct 31, 2024

View reviewed changes

coopp requested a review from bogner October 31, 2024 19:11

damyanp reviewed Nov 1, 2024

View reviewed changes

Cooper Partin added 2 commits November 5, 2024 15:33

Added more tests and updated logic

931a684

Remove setting ComputeShadersPlusRawAndStructuredBuffers flag and tes…

adb6619

…t that went along with it

coopp changed the title ~~Set shader analysis flags EnableRawAndStructuredBuffers and ComputeShadersPlusRawAndStructuredBuffers for RWBuffers~~ Set shader analysis flags EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types Nov 6, 2024

coopp changed the title ~~Set shader analysis flags EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types~~ Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types Nov 6, 2024

tex3d requested changes Nov 6, 2024

View reviewed changes

coopp closed this Nov 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types #114449

Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types #114449

coopp commented Oct 31, 2024

llvmbot commented Oct 31, 2024

coopp commented Oct 31, 2024

tex3d Oct 31, 2024 •

edited

Loading

coopp Oct 31, 2024

tex3d Oct 31, 2024 •

edited

Loading

coopp Nov 5, 2024

damyanp Nov 1, 2024

coopp Nov 1, 2024

damyanp Nov 1, 2024

coopp Nov 6, 2024

tex3d left a comment

tex3d Nov 6, 2024

tex3d Nov 7, 2024

coopp commented Nov 7, 2024

Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types #114449

Set shader analysis flag EnableRawAndStructuredBuffers for RawBuffer and StructuredBuffer resource types #114449

Conversation

coopp commented Oct 31, 2024

llvmbot commented Oct 31, 2024

coopp commented Oct 31, 2024

tex3d Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tex3d Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tex3d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coopp commented Nov 7, 2024

tex3d Oct 31, 2024 •

edited

Loading

tex3d Oct 31, 2024 •

edited

Loading