Add proposal for scalar layout for constant buffers #317

Tobski · 2024-09-12T15:22:17Z

This proposal adds an attribute to specify that the layout of constant buffers uses C-style packing rules, according to the size of the scalar components of matrices and vectors, rather than the packing rules currently used for constant buffers when they are involved.

Tobski · 2024-09-12T15:24:25Z

@microsoft-github-policy-service agree company="AMD"

llvm-beanz · 2024-10-10T21:43:02Z

A few thoughts/questions, mostly around some extra details that would be useful.

Usage

I think you're suggesting we add a source-level attribute something like:

[bufferLayout(legacy)]
cbuffer CB { ... }
[bufferLayout(scalar)]
ConstantBuffer<MyStruct> CBuf;

Is that correct? Should we also have a command line argument to change the default value?
This gets into details that don't yet need to be captured, but we should remember that we'll need to keep track of this in the reflection metadata so that the reflection can communicate the expected layout for a CBV.

Expected Alignments

Can you expand a bit on the expected alignments for different types? Most C & C++ compilers do support vector types with 16-byte alignment, so they wouldn't necessarily be aligned to their element component. How would the new packing rules apply for something like:

struct MyConstants {
  int X;
  int64_t Y[2];
  int16_t Z[3];
  float __attribute__((vector_size(4))) V; // equivalent to float4
};

Tobski · 2024-10-11T15:07:51Z

A few thoughts/questions, mostly around some extra details that would be useful.

Usage

I think you're suggesting we add a source-level attribute something like:
[bufferLayout(legacy)]
cbuffer CB { ... }
[bufferLayout(scalar)]
ConstantBuffer<MyStruct> CBuf;
Is that correct? Should we also have a command line argument to change the default value? This gets into details that don't yet need to be captured, but we should remember that we'll need to keep track of this in the reflection metadata so that the reflection can communicate the expected layout for a CBV.

Yes that's what I'm thinking yep, and I agree a compiler flag to switch the default would be useful.

Expected Alignments

Can you expand a bit on the expected alignments for different types? Most C & C++ compilers do support vector types with 16-byte alignment, so they wouldn't necessarily be aligned to their element component. How would the new packing rules apply for something like:
struct MyConstants {
  int X;
  int64_t Y[2];
  int16_t Z[3];
  float __attribute__((vector_size(4))) V; // equivalent to float4
};

In the Vulkan version of this feature against SPIR-V, vector size doesn't change the alignment requirements; we basically treat them the same as arrays of floats. So the alignment required is that of a float - 4 bytes. I would expect the same here for float4 in HLSL. Granted this is not the same as C/C++ language extensions, but vectors aren't really treated the same way on modern GPUs as they are in CPU ISAs, which is why it works this way in Vulkan (and every supporting vendor's hardware).

I could see an argument for matching C/C++ vector extensions, but it's going to depend on what developers want to see here. We could add in a way to switch between these two packing modes if we think it's helpful though.

Alexander-Johnston · 2025-02-04T17:33:32Z

I've updated this with a first pass to a detailed design. I have the open question of if metadata will be required to determine if the constant buffer is in legacy or scalar layout. Currently when examining the DXIL output of a shader it can be impossible to determine if a constant buffer is in legacy or scalar layout. My (limited) understanding of DX reflection makes me think we do need some metadata to state when a constant buffer is in scalar layout so the runtime can act appropraitely, but I'd like to hear other opinions on this, and if we need to support this or not.

Tobski

Thanks for taking on the detail sections of this proposal, looks good overall, just a few clarifying questions!

Tobski · 2025-02-07T10:21:27Z