From 52590ff2060fdf1225d2826c14a56d1ff026d0fe Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Fri, 19 Jun 2026 19:23:18 -0500 Subject: [PATCH 1/2] [SPIRV] Use scalar rules for BDA alignment Previously the `vk::RawBufferLoad` and `vk::RawBufferStore` instructions used a default alignment of 4 unless the user explicitly specified a different value. This change instead makes the default be based on the minimum legal value for the type being loaded so that users don't need to explicitly override it when loading arbitrary data types. Fixes #8572 Assisted by Claude Opus 4.8 --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 36 +++- ...ntrinsics.vkrawbufferload.64bit-align.hlsl | 154 ++++++++++++++++++ .../intrinsics.vkrawbufferload.hlsl | 4 +- 3 files changed, 186 insertions(+), 8 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.64bit-align.hlsl diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 288b926f24..23a18eea78 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -16231,9 +16231,21 @@ SpirvInstruction *SpirvEmitter::processRawBufferLoad(const CallExpr *callExpr) { return nullptr; } - uint32_t alignment = callExpr->getNumArgs() == 1 - ? 4 - : getRawBufferAlignment(callExpr->getArg(1)); + uint32_t alignment = 0; + if (callExpr->getNumArgs() == 1) { + // Compute the required scalar alignment from the loaded type. + // Per the Vulkan spec, PhysicalStorageBuffer alignment must be at least the + // largest scalar alignment within the type, this matches scalar layout rules. + // See: https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html + AlignmentSizeCalculator alignmentCalc(astContext, spirvOptions); + uint32_t stride = 0; + QualType bufferType = callExpr->getCallReturnType(astContext); + std::tie(alignment, std::ignore) = alignmentCalc.getAlignmentAndSize( + bufferType, SpirvLayoutRule::Scalar, + /*isRowMajor*/ llvm::None, &stride); + } else { + alignment = getRawBufferAlignment(callExpr->getArg(1)); + } if (alignment == 0) return nullptr; @@ -16334,9 +16346,21 @@ SpirvEmitter::processRawBufferStore(const CallExpr *callExpr) { return nullptr; } - uint32_t alignment = callExpr->getNumArgs() == 2 - ? 4 - : getRawBufferAlignment(callExpr->getArg(2)); + uint32_t alignment = 0; + if (callExpr->getNumArgs() == 2) { + // Compute the required scalar alignment from the stored type. + // Per the Vulkan spec, PhysicalStorageBuffer alignment must be at least the + // largest scalar alignment within the type, this matches scalar layout rules. + // See: https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html + QualType bufferType = callExpr->getArg(1)->getType(); + AlignmentSizeCalculator alignmentCalc(astContext, spirvOptions); + uint32_t stride = 0; + std::tie(alignment, std::ignore) = alignmentCalc.getAlignmentAndSize( + bufferType, SpirvLayoutRule::Scalar, + /*isRowMajor*/ llvm::None, &stride); + } else { + alignment = getRawBufferAlignment(callExpr->getArg(2)); + } if (alignment == 0) return nullptr; diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.64bit-align.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.64bit-align.hlsl new file mode 100644 index 0000000000..ea200ad153 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.64bit-align.hlsl @@ -0,0 +1,154 @@ +// RUN: %dxc -T cs_6_2 -E main -enable-16bit-types -fcgl %s -spirv | FileCheck %s + +// Test that vk::RawBufferLoad/Store with 64-bit types uses alignment 8 +// when no explicit alignment is specified. + +struct StructWith64BitMember { + uint64_t member; +}; + +struct StructWithDoubleMember { + double member; +}; + +// Mixes of 16-bit, 32-bit and 64-bit scalar members. The required alignment is +// the largest scalar alignment within the structure. +struct StructWithMixedScalars { + uint16_t a; + uint b; + uint64_t c; +}; + +// A 16-bit/32-bit mix without any 64-bit member: the largest scalar alignment +// is 4, so alignment 4 is used. +struct StructWith16And32BitScalars { + uint16_t a; + float b; +}; + +// A structure with only 16-bit members. The largest scalar alignment is 2, so +// alignment 2 is used: nothing in the Vulkan spec requires a larger minimum. +struct StructWithOnly16BitScalars { + uint16_t a; + half b; +}; + +// A structure containing vectors of 16-bit, 32-bit and 64-bit elements. The +// 64-bit vector forces alignment 8. +struct StructWithMixedVectors { + half2 a; + float3 b; + double2 c; +}; + +// A structure containing matrices. The 64-bit matrix forces alignment 8. +struct StructWithMixedMatrices { + float2x2 a; + double2x2 b; +}; + +uint64_t Address; + +[numthreads(1, 1, 1)] +void main() { + // CHECK: [[addr:%[0-9]+]] = OpLoad %ulong + // CHECK: [[buf:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_ulong + // CHECK-NEXT: [[load:%[0-9]+]] = OpLoad %ulong [[buf]] Aligned 8 + uint64_t scalar = vk::RawBufferLoad(Address); + + // CHECK: [[buf_1:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_double + // CHECK-NEXT: [[load_1:%[0-9]+]] = OpLoad %double [[buf_1]] Aligned 8 + double dbl = vk::RawBufferLoad(Address); + + // CHECK: [[buf_2:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWith64BitMember + // CHECK-NEXT: [[load_2:%[0-9]+]] = OpLoad %StructWith64BitMember{{[^ ]*}} [[buf_2]] Aligned 8 + StructWith64BitMember s1 = vk::RawBufferLoad(Address); + + // CHECK: [[buf_3:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithDoubleMember + // CHECK-NEXT: [[load_3:%[0-9]+]] = OpLoad %StructWithDoubleMember{{[^ ]*}} [[buf_3]] Aligned 8 + StructWithDoubleMember s2 = vk::RawBufferLoad(Address); + + // Stores should also use alignment 8 for 64-bit types. + // CHECK: [[buf_4:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_ulong + // CHECK-NEXT: OpStore [[buf_4]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, scalar); + + // CHECK: [[buf_5:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_double + // CHECK-NEXT: OpStore [[buf_5]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, dbl); + + // CHECK: [[buf_6:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWith64BitMember + // CHECK: OpStore [[buf_6]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, s1); + + // CHECK: [[buf_7:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithDoubleMember + // CHECK: OpStore [[buf_7]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, s2); + + // A structure mixing 16-bit, 32-bit and 64-bit scalars uses alignment 8 + // because of its 64-bit member. + // CHECK: [[buf_8:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedScalars + // CHECK-NEXT: [[load_8:%[0-9]+]] = OpLoad %StructWithMixedScalars{{[^ ]*}} [[buf_8]] Aligned 8 + StructWithMixedScalars s3 = vk::RawBufferLoad(Address); + + // A structure mixing only 16-bit and 32-bit scalars uses alignment 4. + // CHECK: [[buf_9:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWith16And32BitScalars + // CHECK-NEXT: [[load_9:%[0-9]+]] = OpLoad %StructWith16And32BitScalars{{[^ ]*}} [[buf_9]] Aligned 4 + StructWith16And32BitScalars s4 = vk::RawBufferLoad(Address); + + // A structure with only 16-bit scalars has scalar alignment 2, which is the + // minimum required by the Vulkan spec for such an access. + // CHECK: [[buf_10:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithOnly16BitScalars + // CHECK-NEXT: [[load_10:%[0-9]+]] = OpLoad %StructWithOnly16BitScalars{{[^ ]*}} [[buf_10]] Aligned 2 + StructWithOnly16BitScalars s5 = vk::RawBufferLoad(Address); + + // A structure containing 16-bit, 32-bit and 64-bit vectors uses alignment 8 + // because of its 64-bit vector member. + // CHECK: [[buf_11:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedVectors + // CHECK-NEXT: [[load_11:%[0-9]+]] = OpLoad %StructWithMixedVectors{{[^ ]*}} [[buf_11]] Aligned 8 + StructWithMixedVectors s6 = vk::RawBufferLoad(Address); + + // A structure containing 32-bit and 64-bit matrices uses alignment 8 because + // of its 64-bit matrix member. + // CHECK: [[buf_12:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedMatrices + // CHECK-NEXT: [[load_12:%[0-9]+]] = OpLoad %StructWithMixedMatrices{{[^ ]*}} [[buf_12]] Aligned 8 + StructWithMixedMatrices s7 = vk::RawBufferLoad(Address); + + // Stores use the same computed alignments as loads. + // CHECK: [[buf_13:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedScalars + // CHECK: OpStore [[buf_13]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, s3); + + // CHECK: [[buf_14:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWith16And32BitScalars + // CHECK: OpStore [[buf_14]] {{%[0-9]+}} Aligned 4 + vk::RawBufferStore(Address, s4); + + // CHECK: [[buf_15:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithOnly16BitScalars + // CHECK: OpStore [[buf_15]] {{%[0-9]+}} Aligned 2 + vk::RawBufferStore(Address, s5); + + // CHECK: [[buf_16:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedVectors + // CHECK: OpStore [[buf_16]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, s6); + + // CHECK: [[buf_17:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_StructWithMixedMatrices + // CHECK: OpStore [[buf_17]] {{%[0-9]+}} Aligned 8 + vk::RawBufferStore(Address, s7); + + // A 16-bit scalar requires only 2-byte alignment. + // CHECK: [[buf_18:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_ushort + // CHECK-NEXT: [[load_18:%[0-9]+]] = OpLoad %ushort [[buf_18]] Aligned 2 + uint16_t u16 = vk::RawBufferLoad(Address); + + // CHECK: [[buf_19:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_half + // CHECK-NEXT: [[load_19:%[0-9]+]] = OpLoad %half [[buf_19]] Aligned 2 + half h = vk::RawBufferLoad(Address); + + // CHECK: [[buf_20:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_ushort + // CHECK: OpStore [[buf_20]] {{%[0-9]+}} Aligned 2 + vk::RawBufferStore(Address, u16); + + // CHECK: [[buf_21:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_half + // CHECK: OpStore [[buf_21]] {{%[0-9]+}} Aligned 2 + vk::RawBufferStore(Address, h); +} diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl index c2892cfc29..9057c66cbe 100644 --- a/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl +++ b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl @@ -14,7 +14,7 @@ struct BufferData { using MyInt = vk::SpirvType< /*spv::OpTypeInt*/21, - 1,1, // size and alignment + 2,2, // size and alignment (a 16-bit int is 2 bytes) vk::Literal >, // bits vk::Literal > // signed >; @@ -60,7 +60,7 @@ float4 main() : SV_Target0 { d = vk::RawBufferLoad(0); // CHECK: [[buf:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_spirvIntrinsicType %ulong_0 - // CHECK-NEXT: [[load:%[0-9]+]] = OpLoad %spirvIntrinsicType [[buf]] Aligned 4 + // CHECK-NEXT: [[load:%[0-9]+]] = OpLoad %spirvIntrinsicType [[buf]] Aligned 2 // CHECK-NEXT: OpStore %mi [[load]] MyInt mi = vk::RawBufferLoad(0); From b380271eec594c5fb1033f9e5df906c25a6d6b36 Mon Sep 17 00:00:00 2001 From: Chris Bieneman Date: Sat, 20 Jun 2026 10:26:55 -0500 Subject: [PATCH 2/2] clang-format --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 23a18eea78..25e49014e1 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -16235,14 +16235,15 @@ SpirvInstruction *SpirvEmitter::processRawBufferLoad(const CallExpr *callExpr) { if (callExpr->getNumArgs() == 1) { // Compute the required scalar alignment from the loaded type. // Per the Vulkan spec, PhysicalStorageBuffer alignment must be at least the - // largest scalar alignment within the type, this matches scalar layout rules. - // See: https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html + // largest scalar alignment within the type, this matches scalar layout + // rules. See: + // https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html AlignmentSizeCalculator alignmentCalc(astContext, spirvOptions); uint32_t stride = 0; QualType bufferType = callExpr->getCallReturnType(astContext); - std::tie(alignment, std::ignore) = alignmentCalc.getAlignmentAndSize( - bufferType, SpirvLayoutRule::Scalar, - /*isRowMajor*/ llvm::None, &stride); + std::tie(alignment, std::ignore) = + alignmentCalc.getAlignmentAndSize(bufferType, SpirvLayoutRule::Scalar, + /*isRowMajor*/ llvm::None, &stride); } else { alignment = getRawBufferAlignment(callExpr->getArg(1)); } @@ -16350,14 +16351,15 @@ SpirvEmitter::processRawBufferStore(const CallExpr *callExpr) { if (callExpr->getNumArgs() == 2) { // Compute the required scalar alignment from the stored type. // Per the Vulkan spec, PhysicalStorageBuffer alignment must be at least the - // largest scalar alignment within the type, this matches scalar layout rules. - // See: https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html + // largest scalar alignment within the type, this matches scalar layout + // rules. See: + // https://docs.vulkan.org/guide/latest/buffer_device_address_alignment.html QualType bufferType = callExpr->getArg(1)->getType(); AlignmentSizeCalculator alignmentCalc(astContext, spirvOptions); uint32_t stride = 0; - std::tie(alignment, std::ignore) = alignmentCalc.getAlignmentAndSize( - bufferType, SpirvLayoutRule::Scalar, - /*isRowMajor*/ llvm::None, &stride); + std::tie(alignment, std::ignore) = + alignmentCalc.getAlignmentAndSize(bufferType, SpirvLayoutRule::Scalar, + /*isRowMajor*/ llvm::None, &stride); } else { alignment = getRawBufferAlignment(callExpr->getArg(2)); }