Address comments

Author: V-FEXrt

clang/lib/Sema/SemaHLSL.cpp

@@ -174,23 +174,19 @@ Decl *SemaHLSL::ActOnStartBuffer(Scope *BufferScope, bool CBuffer,
 
 static unsigned calculateLegacyCbufferFieldAlign(const ASTContext &Context,
                                                  QualType T) {
-  // Aggregate types are always aligned to new buffer rows
-  if (T->isAggregateType())
+  // 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");
 
-  // 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;
+  // 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
@@ -205,6 +201,14 @@ static unsigned calculateLegacyCbufferSize(const ASTContext &Context,
       QualType Ty = Field->getType();
       unsigned FieldSize = calculateLegacyCbufferSize(Context, Ty);
       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;
     }

clang/test/CodeGenHLSL/cbuffer_align.hlsl

@@ -1,60 +1,71 @@
 // 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:   -fsyntax-only -verify -verify-ignore-unexpected=warning
 
-// 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 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@-1 {{packoffset overlap between 'f0f', 's0f'}}
+}
 
 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
+  float a;
+  double b;
+  float c;
 };
 
-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
-};
+cbuffer CB1Pass {
+  S1 s1p : packoffset(c0.x);
+  float f1p : packoffset(c1.y);
+}
 
-struct S3 {
-  half a;     // 2 bytes + 6 bytes pad or 4 bytes + 4 bytes pad
-  uint64_t b; // 8 bytes
-};
+cbuffer CB1Fail {
+  S1 s1f : packoffset(c0.x);
+  float f1f : packoffset(c1.x);
+  // expected-error@-1 {{packoffset overlap between 'f1f', 's1f'}}
+}
 
-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
+struct S2 {
+  float3 a;
+  float2 b;
 };
 
-
-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 CB2Pass {
+  S2 s2p : packoffset(c0.x);
+  float f2p : packoffset(c1.z);
 }
 
-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 CB2Fail {
+  S2 s2f : packoffset(c0.x);
+  float f2f : packoffset(c1.y);
+  // expected-error@-1 {{packoffset overlap between 'f2f', 's2f'}}
 }
 
-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;
-}
+#if 0
+struct S3 {
+  float3 a;
+  float b;
+};
+
+struct S4 {
+  float2 a;
+  float2 b;
+};
+#endif