[clang] cd4c30b - [HLSL][Sema] Fix Struct Size Calculation containing 16/32 bit scalars (#128086)

[via cfe-commits]

Author: Ashley Coleman
Date: 2025-02-25T17:23:09-07:00
New Revision: cd4c30bb224e432d8cd37f375c138cbaada14f6c

URL: https://github.com/llvm/llvm-project/commit/cd4c30bb224e432d8cd37f375c138cbaada14f6c
DIFF: https://github.com/llvm/llvm-project/commit/cd4c30bb224e432d8cd37f375c138cbaada14f6c.diff

LOG: [HLSL][Sema] Fix Struct Size Calculation containing 16/32 bit scalars (#128086)

Fixes #119641

Update SemaHLSL to correctly calculate the alignment barrier for scalars
that are not 4 bytes wide

Added: 
    clang/test/CodeGenHLSL/cbuffer_align.hlsl

Modified: 
    clang/lib/Sema/SemaHLSL.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index d26d85d5861b1..e4a446f3fe37e 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -172,6 +172,23 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer,
   return Result;
 }
 
+static unsigned calculateLegacyCbufferFieldAlign(const ASTContext &Context,
+                                                 QualType T) {
+  // Arrays and Structs are always aligned to new buffer rows
+  if (T->isArrayType() || T->isStructureType())
+    return 16;
+
+  // Vectors are aligned to the type they contain
+  if (const VectorType *VT = T->getAs<VectorType>())
+    return calculateLegacyCbufferFieldAlign(Context, VT->getElementType());
+
+  assert(Context.getTypeSize(T) <= 64 &&
+         "Scalar bit widths larger than 64 not supported");
+
+  // Scalar types are aligned to their byte width
+  return Context.getTypeSize(T) / 8;
+}
+
 // Calculate the size of a legacy cbuffer type in bytes based on
 // https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-packing-rules
 static unsigned calculateLegacyCbufferSize(const ASTContext &Context,
@@ -183,11 +200,15 @@ static unsigned calculateLegacyCbufferSize(const ASTContext &Context,
     for (const FieldDecl *Field : RD->fields()) {
       QualType Ty = Field->getType();
       unsigned FieldSize = calculateLegacyCbufferSize(Context, Ty);
-      // FIXME: This is not the correct alignment, it does not work for 16-bit
-      // types. See llvm/llvm-project#119641.
-      unsigned FieldAlign = 4;
-      if (Ty->isAggregateType())
+      unsigned FieldAlign = calculateLegacyCbufferFieldAlign(Context, Ty);
+
+      // If the field crosses the row boundary after alignment it drops to the
+      // next row
+      unsigned AlignSize = llvm::alignTo(Size, FieldAlign);
+      if ((AlignSize % CBufferAlign) + FieldSize > CBufferAlign) {
         FieldAlign = CBufferAlign;
+      }
+
       Size = llvm::alignTo(Size, FieldAlign);
       Size += FieldSize;
     }

diff  --git a/clang/test/CodeGenHLSL/cbuffer_align.hlsl b/clang/test/CodeGenHLSL/cbuffer_align.hlsl
new file mode 100644
index 0000000000000..25fe20da7a230
--- /dev/null
+++ b/clang/test/CodeGenHLSL/cbuffer_align.hlsl
@@ -0,0 +1,121 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -fsyntax-only -verify -verify-ignore-unexpected=warning
+
+struct S0 {
+  half a;
+  half b;
+  half c;
+  half d;
+  half e;
+  half f;
+  half g;
+  half h;
+};
+
+cbuffer CB0Pass {
+  S0 s0p : packoffset(c0.x);
+  float f0p : packoffset(c1.x);
+}
+
+cbuffer CB0Fail {
+  S0 s0f : packoffset(c0.x);
+  float f0f : packoffset(c0.w);
+  // expected-error at -1 {{packoffset overlap between 'f0f', 's0f'}}
+}
+
+struct S1 {
+  float a;
+  double b;
+  float c;
+};
+
+cbuffer CB1Pass {
+  S1 s1p : packoffset(c0.x);
+  float f1p : packoffset(c1.y);
+}
+
+cbuffer CB1Fail {
+  S1 s1f : packoffset(c0.x);
+  float f1f : packoffset(c1.x);
+  // expected-error at -1 {{packoffset overlap between 'f1f', 's1f'}}
+}
+
+struct S2 {
+  float3 a;
+  float2 b;
+};
+
+cbuffer CB2Pass {
+  S2 s2p : packoffset(c0.x);
+  float f2p : packoffset(c1.z);
+}
+
+cbuffer CB2Fail {
+  S2 s2f : packoffset(c0.x);
+  float f2f : packoffset(c1.y);
+  // expected-error at -1 {{packoffset overlap between 'f2f', 's2f'}}
+}
+
+struct S3 {
+  float3 a;
+  float b;
+};
+
+cbuffer CB3Pass {
+  S3 s3p : packoffset(c0.x);
+  float f3p : packoffset(c1.x);
+}
+
+cbuffer CB3Fail {
+  S3 s3f : packoffset(c0.x);
+  float f3f : packoffset(c0.w);
+  // expected-error at -1 {{packoffset overlap between 'f3f', 's3f'}}
+}
+
+struct S4 {
+  float2 a;
+  float2 b;
+};
+
+cbuffer CB4Pass {
+  S4 s4p : packoffset(c0.x);
+  float f4p : packoffset(c1.x);
+}
+
+cbuffer CB4Fail {
+  S4 s4f : packoffset(c0.x);
+  float f4f : packoffset(c0.w);
+  // expected-error at -1 {{packoffset overlap between 'f4f', 's4f'}}
+}
+
+struct S5 {
+  float a[3];
+};
+
+cbuffer CB5Pass {
+  S5 s5p : packoffset(c0.x);
+  float f5p : packoffset(c2.y);
+}
+
+cbuffer CB5Fail {
+  S5 s5f : packoffset(c0.x);
+  float f5f : packoffset(c2.x);
+  // expected-error at -1 {{packoffset overlap between 'f5f', 's5f'}}
+}
+
+struct S6 {
+  float a;
+  float2 b;
+};
+
+cbuffer CB6Pass {
+  S6 s6p : packoffset(c0.x);
+  float f6p : packoffset(c0.w);
+}
+
+cbuffer CB6Fail {
+  S6 s6f : packoffset(c0.x);
+  float f6f : packoffset(c0.y);
+  // expected-error at -1 {{packoffset overlap between 'f6f', 's6f'}}
+}