DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

-not_use_legacy_cbuf_load and linear cbuffer layout

Open TheRealMJP opened this issue 5 years ago • 11 comments

Hey everyone,

I'm a little confused about the -not_use_legacy_cbuf_load command line option, and how it ties into cbuffer layout. Hopefully someone can help clear up what the intention here is around that particular flag.

The docs here suggest that there's "linear" cbuffer layout that diverges from the old DXBC behavior. I was hoping that this layout is something we could use to finally be free of the old "vectors can't cross 16-byte boundaries" alignment requirements that we've had since DX10, which also aligns array elements to 16-byte boundaries. I was also hoping that the -not_use_legacy_cbuf_load flag would allow opting-in to linear cbuffer layout, however this doesn't seem to be the case when I use it on a simple pixel shader:

struct CBLayout
{
    float floats[4];    
    float scalar;
    float4 f4;
};

ConstantBuffer<CBLayout> CB;

struct PSInput
{
	float4 color : COLOR;
};

float4 PSMain(PSInput input) : SV_TARGET
{
	return input.color * CB.floats[0] * CB.floats[1] * CB.scalar * CB.f4;
}
; Buffer Definitions:
;
; cbuffer CB
; {
;
;   struct CB
;   {
;
;       struct struct.CBLayout
;       {
;
;           float floats[4];                          ; Offset:    0
;           float scalar;                             ; Offset:   52
;           float4 f4;                                ; Offset:   64
;       
;       } CB;                                         ; Offset:    0
;
;   
;   } CB;                                             ; Offset:    0 Size:    80
;
; }

  %5 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 0, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %.i0 = fmul fast float %5, %1
  %.i1 = fmul fast float %5, %2
  %.i2 = fmul fast float %5, %3
  %.i3 = fmul fast float %5, %4
  %6 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 16, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %.i01 = fmul fast float %.i0, %6
  %.i12 = fmul fast float %.i1, %6
  %.i23 = fmul fast float %.i2, %6
  %.i34 = fmul fast float %.i3, %6
  %7 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 52, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %.i05 = fmul fast float %.i01, %7
  %.i16 = fmul fast float %.i12, %7
  %.i27 = fmul fast float %.i23, %7
  %.i38 = fmul fast float %.i34, %7
  %8 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 64, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %9 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 68, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %10 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 72, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %11 = call float @dx.op.cbufferLoad.f32(i32 58, %dx.types.Handle %CB_cbuffer, i32 76, i32 8)  ; CBufferLoad(handle,byteOffset,alignment)
  %.i09 = fmul fast float %.i05, %8
  %.i110 = fmul fast float %.i16, %9
  %.i211 = fmul fast float %.i27, %10
  %.i312 = fmul fast float %.i38, %11

Is the intention here to keep the old DXBC packing behavior even when using the non-legacy cbuffer loads? Or is this just a bug/oversight? If this is intentional, I would cast my vote for adding a new flag that allows for scalar/linear layout in constant buffers. The odd alignment rules are still common source of bugs among new programmers, and it would be much easier for them if we could use scalar packing rules similar to what's exposed in Vulkan with their "scalar block layout" feature.

Thanks in advance!

-Matt

TheRealMJP avatar Oct 19 '19 19:10 TheRealMJP

The intention is to change to match StructuredBuffer layout for the new cbufferLoad. However, there are various known bugs when enabling this option, and there's a PR that needs a little more work before merging that should fix them.

tex3d avatar Oct 24 '19 01:10 tex3d

Thank you Tex, that answers my questions. I'll definitely be looking forward to that PR! 🙂

TheRealMJP avatar Oct 25 '19 06:10 TheRealMJP

Reopening this issue to make sure that this get fixed when I pull in my change.

vcsharma avatar Sep 14 '20 22:09 vcsharma

Related to #972

vcsharma avatar Mar 04 '21 18:03 vcsharma

Is there any hope of this getting fixed in the near future? I am currently working on a project where it would be very benificial to have -no-legacy-cbuf-layout work and actually have the same layout for constant buffers as we have for structured buffers.

mcbouterse avatar Nov 22 '21 13:11 mcbouterse

Hi @tex3d @vcsharma, I'm also very very curious about this :) Currently attempts to use it on a simple cbuffer lead to a weird error

CreateComputePipelineState failed: [0x8007000e] Not enough memory resources are available to complete this operation.

Thanks!

curldivergence avatar Dec 04 '21 10:12 curldivergence

Hi DXC team!

I was curious if there was any progress on this front since this flag (if it worked) would be of great help for those of us that still have cbuffer usage.

jeremyong-az avatar Sep 20 '22 14:09 jeremyong-az

@tex3d Any comment on this thread, is it better for users to abandon cbuffers and close this issue, or is it planned in the future to fix it?

mathforlife83 avatar Sep 26 '22 08:09 mathforlife83

In the meantime @tex3d it may be worth editing the wiki here to indicate that this flag shouldn't be used in the meantime (and ideally emit a warning/error if the flag is used). I'm sure attempting to experiment with the flag at the moment has caused some developers to go down an unfortunate rabbit hole if they expect the flag to work as currently documented.

jeremyong avatar Jul 25 '23 19:07 jeremyong

@tex3d & @pow2clk, given the complications of fixing this in the short term, should we just remove this flag until we put the work on the roadmap?

llvm-beanz avatar Jul 25 '23 20:07 llvm-beanz

I filed #6306 to suggest deprecating and removing the flag. If we do that we should go through and clean up any issues (like this one) that report issues with it.

llvm-beanz avatar Feb 14 '24 21:02 llvm-beanz

As noted in #6306 we've deprecated this flag.

damyanp avatar Jun 19 '24 17:06 damyanp