From 8b3e3beea66d9773adf11ea2e163577d649f3d7c Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Tue, 28 Jan 2020 12:35:13 -0800 Subject: Fix layout for structured buffers of matrices (#1184) When using row-major layout (via command-line or API option), the following sort of declaration: ```hlsl StructuredBuffer gBuffer; ... gBuffer[i] ... ``` Generates unexpected results when compiled to DXBC via fxc or DXIL via dxc, because the fxc/dxc compilers do not respect the matrix layout mode in this specific case (a structured buffer of matrices). Instead, they always use column-major layout, even if row-major was requested by the user. A user can work around this behavior by wrapping the matrix in a `struct`: ```hlsl struct Wrapper { float4x4 wrapped; } SturcturedBuffer gBuffer; ... gBuffer[i].wrapped ... ``` This change simply automates that workaround when compiling for an HLSL-based downstream compiler, so that we get the same behavior across all our backends. The change adds a test case to confirm the behavior across multiple targets, but it turns out we also had a test checked in that confirmed the buggy (or at least surprising) fxc/dxc behavior, so that one had its baselines changed and can work as a regression test for this fix as well. --- .../compute/matrix-layout-structured-buffer.slang | 17 +++------ ...x-layout-structured-buffer.slang.1.expected.txt | 12 +++++++ ...x-layout-structured-buffer.slang.2.expected.txt | 12 +++++++ ...x-layout-structured-buffer.slang.3.expected.txt | 12 +++++++ ...rix-layout-structured-buffer.slang.expected.txt | 24 ++++++------- tests/compute/structured-buffer-of-matrices.slang | 41 ++++++++++++++++++++++ ...tructured-buffer-of-matrices.slang.expected.txt | 4 +++ 7 files changed, 97 insertions(+), 25 deletions(-) create mode 100644 tests/compute/matrix-layout-structured-buffer.slang.1.expected.txt create mode 100644 tests/compute/matrix-layout-structured-buffer.slang.2.expected.txt create mode 100644 tests/compute/matrix-layout-structured-buffer.slang.3.expected.txt create mode 100644 tests/compute/structured-buffer-of-matrices.slang create mode 100644 tests/compute/structured-buffer-of-matrices.slang.expected.txt (limited to 'tests') diff --git a/tests/compute/matrix-layout-structured-buffer.slang b/tests/compute/matrix-layout-structured-buffer.slang index 36596424d..bb7dbb381 100644 --- a/tests/compute/matrix-layout-structured-buffer.slang +++ b/tests/compute/matrix-layout-structured-buffer.slang @@ -1,18 +1,9 @@ // matrix-layout-structured-buffer.slang -// This test is set up to confirm that `StructuredBuffer` types are -// always laid out column-major by fxc/dxc, even when row-major layout has been -// requested globally. -// -// This behavior should be considered a bug in Slang because either: -// -// 1. we should report reflection layout information that acknowledges this behavior, or -// 2. we should alter our HLSL output passed to fxc/dxc to provide consistent -// behavior that matches our reflection data. -// -// For now this test exists to document the situation. It's output can/should -// be updated if we decide to fix the underlying problem by taking option (2). -// +// This test confirms that we apply the matrix layout +// mode requested by the user, even in the case of structured +// buffers of matrices, where fxc/dxc do *not* respect +// the matrix layout mode by default. //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -xslang -matrix-layout-row-major //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -xslang -matrix-layout-column-major diff --git a/tests/compute/matrix-layout-structured-buffer.slang.1.expected.txt b/tests/compute/matrix-layout-structured-buffer.slang.1.expected.txt new file mode 100644 index 000000000..18d8be654 --- /dev/null +++ b/tests/compute/matrix-layout-structured-buffer.slang.1.expected.txt @@ -0,0 +1,12 @@ +80010300 +90111301 +88060B02 +95160C03 +81020404 +910F1405 +86070906 +96170D07 +82000508 +8F101209 +87080A0A +97150E0B diff --git a/tests/compute/matrix-layout-structured-buffer.slang.2.expected.txt b/tests/compute/matrix-layout-structured-buffer.slang.2.expected.txt new file mode 100644 index 000000000..fd74697bb --- /dev/null +++ b/tests/compute/matrix-layout-structured-buffer.slang.2.expected.txt @@ -0,0 +1,12 @@ +80040100 +91151201 +8A020B02 +8F130C03 +84080504 +950D1605 +82060306 +93171007 +88000908 +8D110E09 +860A070A +970F140B \ No newline at end of file diff --git a/tests/compute/matrix-layout-structured-buffer.slang.3.expected.txt b/tests/compute/matrix-layout-structured-buffer.slang.3.expected.txt new file mode 100644 index 000000000..18d8be654 --- /dev/null +++ b/tests/compute/matrix-layout-structured-buffer.slang.3.expected.txt @@ -0,0 +1,12 @@ +80010300 +90111301 +88060B02 +95160C03 +81020404 +910F1405 +86070906 +96170D07 +82000508 +8F101209 +87080A0A +97150E0B diff --git a/tests/compute/matrix-layout-structured-buffer.slang.expected.txt b/tests/compute/matrix-layout-structured-buffer.slang.expected.txt index 18d8be654..e0bbef746 100644 --- a/tests/compute/matrix-layout-structured-buffer.slang.expected.txt +++ b/tests/compute/matrix-layout-structured-buffer.slang.expected.txt @@ -1,12 +1,12 @@ -80010300 -90111301 -88060B02 -95160C03 -81020404 -910F1405 -86070906 -96170D07 -82000508 -8F101209 -87080A0A -97150E0B +80040100 +91151201 +8A020B02 +8F130C03 +84080504 +950D1605 +82060306 +93171007 +88000908 +8D110E09 +860A070A +970F140B diff --git a/tests/compute/structured-buffer-of-matrices.slang b/tests/compute/structured-buffer-of-matrices.slang new file mode 100644 index 000000000..e87cf8c59 --- /dev/null +++ b/tests/compute/structured-buffer-of-matrices.slang @@ -0,0 +1,41 @@ +// structured-buffer-of-matrices.slang + +// This test confirms that we have implemented a workaround +// for the issue where the Microsoft HLSL compilers (fxc and dxc) +// ignore matrix layout settings (like `#pragma pack_matrx`) +// in the case where a matrix type is used directly in a +// structured buffer (e.g., `StructuredBuffer`), +// and instead always use column-major layout. +// +// With our fix in place, this test should produce the same +// output on all targets, correctly respecting the user's +// request for row-major layout via command-line option. + +//TEST(compute):COMPARE_COMPUTE_EX:-cpu -compute -compile-arg -O3 -xslang -matrix-layout-row-major +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -xslang -matrix-layout-row-major +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 -xslang -matrix-layout-row-major +//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute -xslang -matrix-layout-row-major + +// Note: we are using a buffer of floating-point matrices, but fill it with integer +// data, to allow this test to work on the Vulkan targets, which do not currently +// support integer matrices, because of GLSL limitations. + +//TEST_INPUT:ubuffer(data=[0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15], stride=64):name gInput +RWStructuredBuffer gInput; + + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name gOutput +RWStructuredBuffer gOutput; + +int test(int val) +{ + return asint(gInput[0][(val+1)&3][(val+3)&3]); +} + +[numthreads(4, 1, 1)] +void computeMain(uint3 tid : SV_DispatchThreadID) +{ + int value = tid.x; + value = test(value); + gOutput[tid.x] = value; +} \ No newline at end of file diff --git a/tests/compute/structured-buffer-of-matrices.slang.expected.txt b/tests/compute/structured-buffer-of-matrices.slang.expected.txt new file mode 100644 index 000000000..1e59fb2f4 --- /dev/null +++ b/tests/compute/structured-buffer-of-matrices.slang.expected.txt @@ -0,0 +1,4 @@ +7 +8 +D +2 -- cgit v1.2.3