Skip to content

[DXIL][Analysis] Make alignment on StructuredBuffer optional #100697

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

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Jul 26, 2024

HLSL allows StructuredBuffer<> to be defined with scalar or
up-to-4-element vectors as well as with structs, but when doing so
dxc doesn't set the alignment. Emulate this.

bogner added 2 commits July 25, 2024 22:45
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

HLSL allows StructuredBuffer<> to be defined with scalar or
up-to-4-element vectors as well as with structs, but when doing so
dxc doesn't set the alignment. Emulate this behaviour.


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

3 Files Affected:

  • (modified) llvm/include/llvm/Analysis/DXILResource.h (+6-6)
  • (modified) llvm/lib/Analysis/DXILResource.cpp (+6-3)
  • (modified) llvm/unittests/Analysis/DXILResourceTest.cpp (+13)
diff --git a/llvm/include/llvm/Analysis/DXILResource.h b/llvm/include/llvm/Analysis/DXILResource.h
index dff0b7cfcd142..0fad598d416ec 100644
--- a/llvm/include/llvm/Analysis/DXILResource.h
+++ b/llvm/include/llvm/Analysis/DXILResource.h
@@ -47,7 +47,7 @@ class ResourceInfo {
 
   struct StructInfo {
     uint32_t Stride;
-    Align Alignment;
+    MaybeAlign Alignment;
 
     bool operator==(const StructInfo &RHS) const {
       return std::tie(Stride, Alignment) == std::tie(RHS.Stride, RHS.Alignment);
@@ -140,7 +140,7 @@ class ResourceInfo {
   void setSampler(dxil::SamplerType Ty) {
     SamplerTy = Ty;
   }
-  void setStruct(uint32_t Stride, Align Alignment) {
+  void setStruct(uint32_t Stride, MaybeAlign Alignment) {
     assert(isStruct() && "Not a Struct");
     Struct.Stride = Stride;
     Struct.Alignment = Alignment;
@@ -166,7 +166,7 @@ class ResourceInfo {
                           dxil::ResourceKind Kind);
   static ResourceInfo RawBuffer(Value *Symbol, StringRef Name);
   static ResourceInfo StructuredBuffer(Value *Symbol, StringRef Name,
-                                       uint32_t Stride, Align Alignment);
+                                       uint32_t Stride, MaybeAlign Alignment);
   static ResourceInfo Texture2DMS(Value *Symbol, StringRef Name,
                                   dxil::ElementType ElementTy,
                                   uint32_t ElementCount, uint32_t SampleCount);
@@ -182,9 +182,9 @@ class ResourceInfo {
   static ResourceInfo RWRawBuffer(Value *Symbol, StringRef Name,
                                   bool GloballyCoherent, bool IsROV);
   static ResourceInfo RWStructuredBuffer(Value *Symbol, StringRef Name,
-                                         uint32_t Stride,
-                                         Align Alignment, bool GloballyCoherent,
-                                         bool IsROV, bool HasCounter);
+                                         uint32_t Stride, MaybeAlign Alignment,
+                                         bool GloballyCoherent, bool IsROV,
+                                         bool HasCounter);
   static ResourceInfo RWTexture2DMS(Value *Symbol, StringRef Name,
                                     dxil::ElementType ElementTy,
                                     uint32_t ElementCount, uint32_t SampleCount,
diff --git a/llvm/lib/Analysis/DXILResource.cpp b/llvm/lib/Analysis/DXILResource.cpp
index 1443fef8fc165..d49fed7f72465 100644
--- a/llvm/lib/Analysis/DXILResource.cpp
+++ b/llvm/lib/Analysis/DXILResource.cpp
@@ -79,7 +79,8 @@ ResourceInfo ResourceInfo::RawBuffer(Value *Symbol, StringRef Name) {
 }
 
 ResourceInfo ResourceInfo::StructuredBuffer(Value *Symbol, StringRef Name,
-                                            uint32_t Stride, Align Alignment) {
+                                            uint32_t Stride,
+                                            MaybeAlign Alignment) {
   ResourceInfo RI(ResourceClass::SRV, ResourceKind::StructuredBuffer, Symbol,
                   Name);
   RI.setStruct(Stride, Alignment);
@@ -127,7 +128,8 @@ ResourceInfo ResourceInfo::RWRawBuffer(Value *Symbol, StringRef Name,
 }
 
 ResourceInfo ResourceInfo::RWStructuredBuffer(Value *Symbol, StringRef Name,
-                                              uint32_t Stride, Align Alignment,
+                                              uint32_t Stride,
+                                              MaybeAlign Alignment,
                                               bool GloballyCoherent, bool IsROV,
                                               bool HasCounter) {
   ResourceInfo RI(ResourceClass::UAV, ResourceKind::StructuredBuffer, Symbol,
@@ -284,7 +286,8 @@ MDTuple *ResourceInfo::getAsMetadata(LLVMContext &Ctx) const {
 
 std::pair<uint32_t, uint32_t> ResourceInfo::getAnnotateProps() const {
   uint32_t ResourceKind = llvm::to_underlying(Kind);
-  uint32_t AlignLog2 = isStruct() ? Log2(Struct.Alignment) : 0;
+  uint32_t AlignLog2 =
+      isStruct() && Struct.Alignment ? Log2(*Struct.Alignment) : 0;
   bool IsUAV = isUAV();
   bool IsROV = IsUAV && UAVFlags.IsROV;
   bool IsGloballyCoherent = IsUAV && UAVFlags.GloballyCoherent;
diff --git a/llvm/unittests/Analysis/DXILResourceTest.cpp b/llvm/unittests/Analysis/DXILResourceTest.cpp
index 554cbd0d8ded7..7bbb417097882 100644
--- a/llvm/unittests/Analysis/DXILResourceTest.cpp
+++ b/llvm/unittests/Analysis/DXILResourceTest.cpp
@@ -151,6 +151,19 @@ TEST(DXILResource, AnnotationsAndMetadata) {
   EXPECT_MDEQ(
       MD, TestMD.get(0, Symbol, "Buffer0", 0, 0, 1, 12, 0, TestMD.get(1, 16)));
 
+  // StructuredBuffer<float3> Buffer1 : register(t1);
+  Symbol = UndefValue::get(StructType::create(
+      Context, {Floatx3Ty}, "class.StructuredBuffer<vector<float, 3> >"));
+  Resource = ResourceInfo::StructuredBuffer(Symbol, "Buffer1",
+                                            /*Stride=*/12, {});
+  Resource.bind(1, 0, 1, 1);
+  Props = Resource.getAnnotateProps();
+  EXPECT_EQ(Props.first, 0x0000000cU);
+  EXPECT_EQ(Props.second, 0x0000000cU);
+  MD = Resource.getAsMetadata(Context);
+  EXPECT_MDEQ(
+      MD, TestMD.get(1, Symbol, "Buffer1", 0, 1, 1, 12, 0, TestMD.get(1, 12)));
+
   // Texture2D<float4> ColorMapTexture : register(t2);
   Symbol = UndefValue::get(StructType::create(
       Context, {Floatx4Ty}, "class.Texture2D<vector<float, 4> >"));

@farzonl
Copy link
Member

farzonl commented Jul 26, 2024

HLSL allows StructuredBuffer<> to be defined with scalar or up-to-4-element vectors as well as with structs, but when doing so dxc doesn't set the alignment. Emulate this behaviour.

*behavior

bogner added 4 commits July 27, 2024 22:26
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@bogner bogner changed the base branch from users/bogner/sprmain.dxilanalysis-make-alignment-on-structuredbuffer-optional to main July 29, 2024 21:41
@bogner bogner merged commit a94edb6 into main Jul 29, 2024
5 of 7 checks passed
@bogner bogner deleted the users/bogner/sprdxilanalysis-make-alignment-on-structuredbuffer-optional branch July 29, 2024 21:41
bogner added a commit that referenced this pull request Jul 29, 2024
…#101088)

Seeing build failures, reverting to investigate.

Reverts #100697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants