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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Ashley Coleman (V-FEXrt)

<details>
<summary>Changes</summary>

Fixes #<!-- -->119641

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

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaHLSL.cpp (+22-5) 
- (added) clang/test/CodeGenHLSL/cbuffer_align.hlsl (+60) 


``````````diff
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index d26d85d5861b1..cccfd4aec7eb2 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -172,6 +172,27 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer,
   return Result;
 }
 
+static unsigned calculateLegacyCbufferFieldAlign(const ASTContext &Context,
+                                                 QualType T) {
+  // Aggregate types are always aligned to new buffer rows
+  if (T->isAggregateType())
+    return 16;
+
+  assert(Context.getTypeSize(T) <= 64 &&
+         "Scalar bit widths larger than 64 not supported");
+
+  // 64 bit types such as double and uint64_t align to 8 bytes
+  if (Context.getTypeSize(T) == 64)
+    return 8;
+
+  // Half types align to 2 bytes only if native half is available
+  if (T->isHalfType() && Context.getLangOpts().NativeHalfType)
+    return 2;
+
+  // Everything else aligns to 4 bytes
+  return 4;
+}
+
 // 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 +204,7 @@ 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())
-        FieldAlign = CBufferAlign;
+      unsigned FieldAlign = calculateLegacyCbufferFieldAlign(Context, Ty);
       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..31db2e0726020
--- /dev/null
+++ b/clang/test/CodeGenHLSL/cbuffer_align.hlsl
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s --check-prefix=CHECK-HALF
+
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s \
+// RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s --check-prefix=CHECK-FLOAT
+
+
+struct S1 {
+  half a;   // 2 bytes + 2 bytes pad or 4 bytes
+  float b;  // 4 bytes
+  half c;   // 2 bytes + 2 bytes pad or 4 bytes
+  float d;  // 4 bytes
+  double e; // 8 bytes
+};
+
+struct S2 {
+  half a;   // 2 bytes or 4 bytes
+  half b;   // 2 bytes or 4 bytes
+  float e;  // 4 bytes or 4 bytes + 4 padding
+  double f; // 8 bytes
+};
+
+struct S3 {
+  half a;     // 2 bytes + 6 bytes pad or 4 bytes + 4 bytes pad
+  uint64_t b; // 8 bytes
+};
+
+struct S4 {
+  float a;  // 4 bytes
+  half b;   // 2 bytes or 4 bytes
+  half c;   // 2 bytes or 4 bytes + 4 bytes pad
+  double d; // 8 bytes
+};
+
+
+cbuffer CB0 {
+  // CHECK-HALF: @CB0.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB0, 24, 0))
+  // CHECK-FLOAT: @CB0.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB0, 24, 0))
+  S1 s1;
+}
+
+cbuffer CB1 {
+  // CHECK-HALF: @CB1.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB1, 16, 0))
+  // CHECK-FLOAT: @CB1.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB1, 24, 0))
+  S2 s2;
+}
+
+cbuffer CB2 {
+  // CHECK-HALF: @CB2.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB2, 16, 0))
+  // CHECK-FLOAT: @CB2.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB2, 16, 0))
+  S3 s3;
+}
+
+cbuffer CB3 {
+  // CHECK-HALF: @CB3.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB3, 16, 0))
+  // CHECK-FLOAT: @CB3.cb = external constant target("dx.CBuffer", target("dx.Layout", %__cblayout_CB3, 24, 0))
+  S4 s4;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/128086