Browse Source

Set the layout rule in processTemplatedLoadFromBuffer (#5192)

* Set the layout rule in processTemplatedLoadFromBuffer

They layout rules for value coming from processTemplatedLoadFromBuffer
are inconsistent, and this is causing problems. This PR changes the
function to return an instruction whose layout rule is Void.

This should remove the inconsistenty.

Fixes #5123

* Add test

The cg.return.memory-layout.hlsl had to be updated because the Byteaddress load no longer has a different layout rule.

* Remove `-Vd` from test
Steven Perron 2 years ago
parent
commit
fae8401702

+ 14 - 8
tools/clang/lib/SPIRV/RawBufferMethods.cpp

@@ -200,22 +200,23 @@ SpirvInstruction *RawBufferHandler::processTemplatedLoadFromBuffer(
 
   // Scalar types
   if (isScalarType(targetType)) {
+    SpirvInstruction *scalarResult = nullptr;
     auto loadWidth = getElementSpirvBitwidth(
         astContext, targetType, theEmitter.getSpirvOptions().enable16BitTypes);
     switch (bitOffset) {
     case 0: {
       switch (loadWidth) {
       case 16:
-        return load16BitsAtBitOffset0(buffer, index, targetType, bitOffset,
-                                      range);
+        scalarResult =
+            load16BitsAtBitOffset0(buffer, index, targetType, bitOffset, range);
         break;
       case 32:
-        return load32BitsAtBitOffset0(buffer, index, targetType, bitOffset,
-                                      range);
+        scalarResult =
+            load32BitsAtBitOffset0(buffer, index, targetType, bitOffset, range);
         break;
       case 64:
-        return load64BitsAtBitOffset0(buffer, index, targetType, bitOffset,
-                                      range);
+        scalarResult =
+            load64BitsAtBitOffset0(buffer, index, targetType, bitOffset, range);
         break;
       default:
         theEmitter.emitError(
@@ -229,8 +230,8 @@ SpirvInstruction *RawBufferHandler::processTemplatedLoadFromBuffer(
     case 16: {
       switch (loadWidth) {
       case 16:
-        return load16BitsAtBitOffset16(buffer, index, targetType, bitOffset,
-                                       range);
+        scalarResult = load16BitsAtBitOffset16(buffer, index, targetType,
+                                               bitOffset, range);
         break;
       case 32:
       case 64:
@@ -255,6 +256,11 @@ SpirvInstruction *RawBufferHandler::processTemplatedLoadFromBuffer(
           loc);
       return nullptr;
     }
+    assert(scalarResult != nullptr);
+    // We set the layout rule for scalars. Other types are built up from the
+    // scalars, and should inherit this layout rule or default to Void.
+    scalarResult->setLayoutRule(SpirvLayoutRule::Void);
+    return scalarResult;
   }
 
   // Vector types

+ 3 - 1
tools/clang/lib/SPIRV/RawBufferMethods.h

@@ -28,7 +28,9 @@ public:
   /// which is a runtime array in SPIR-V. This method works by loading one or
   /// more uints, and performing necessary casts and composite constructions
   /// to build the 'targetType'. The 'offset' parameter can be used for finer
-  /// grained load of bitwidths smaller than 32-bits.
+  /// grained load of bitwidths smaller than 32-bits. The layout rule for the
+  /// result will be `Void` because the value will be built and used internally
+  /// only. It does not have to match `buffer`.
   ///
   /// Example:
   /// targetType = uint16_t, address=0, offset=0

+ 10 - 9
tools/clang/test/CodeGenSPIRV/cf.return.memory-layout.hlsl

@@ -8,28 +8,29 @@ struct Data
     uint d;
 };
 
-RWByteAddressBuffer buffer;
+RWStructuredBuffer<Data> buffer;
+
 
 // CHECK:     OpName [[Data:%\w+]] "Data"
 // CHECK-NOT: OpMemberDecorate [[Data]] 0 Offset 0
 // CHECK:     OpName [[Data_0:%\w+]] "Data"
-// CHECK-NOT: OpMemberDecorate [[Data]] 0 Offset 0
-// CHECK:     OpMemberDecorate [[Data_0]] 0 Offset 0
-// CHECK:     OpMemberDecorate [[Data_0]] 1 Offset 4
-// CHECK:     OpMemberDecorate [[Data_0]] 2 Offset 8
-// CHECK:     OpMemberDecorate [[Data_0]] 3 Offset 12
+// CHECK-NOT: OpMemberDecorate [[Data_0]] 0 Offset 0
+// CHECK:     OpMemberDecorate [[Data]] 0 Offset 0
+// CHECK:     OpMemberDecorate [[Data]] 1 Offset 4
+// CHECK:     OpMemberDecorate [[Data]] 2 Offset 8
+// CHECK:     OpMemberDecorate [[Data]] 3 Offset 12
 // CHECK:     [[Data]] = OpTypeStruct %uint %uint %uint %uint
 // CHECK:     [[Data_0]] = OpTypeStruct %uint %uint %uint %uint
 
 Data returnDataWithoutPhysicalMemoryLayout(uint idx)
 {
-// CHECK: [[comp:%\d+]] = OpCompositeConstruct [[Data_0]]
+// CHECK: [[comp:%\d+]] = OpLoad [[Data]]
 // CHECK: [[a:%\d+]] = OpCompositeExtract %uint [[comp]] 0
 // CHECK: [[b:%\d+]] = OpCompositeExtract %uint [[comp]] 1
 // CHECK: [[c:%\d+]] = OpCompositeExtract %uint [[comp]] 2
 // CHECK: [[d:%\d+]] = OpCompositeExtract %uint [[comp]] 3
-// CHECK: OpCompositeConstruct [[Data]] [[a]] [[b]] [[c]] [[d]]
-    return buffer.Load<Data>(idx);
+// CHECK: OpCompositeConstruct [[Data_0]] [[a]] [[b]] [[c]] [[d]]
+    return buffer[idx];
 }
 
 [numthreads(1, 1, 1)]

+ 23 - 0
tools/clang/test/CodeGenSPIRV/method.byte-address-buffer.load.layout.hlsl

@@ -0,0 +1,23 @@
+// RUN: %dxc -spirv -T lib_6_5 -E AnyHit1 -fspv-target-env=vulkan1.3 -Wno-effects-syntax -fcgl %s | FileCheck %s
+
+// CHECK-NOT: error
+struct DataType
+{
+	uint u;
+	bool b;
+};
+
+static DataType data;
+
+ByteAddressBuffer buffer : register ( t4 , space3 )  ; 
+
+struct PayloadShadow_t 
+{ 
+    float m_flVisibility ; 
+} ; 
+
+[ shader ( "anyhit" ) ] 
+void AnyHit1 ( inout PayloadShadow_t payload , in BuiltInTriangleIntersectionAttributes attrs ) 
+{
+	data = buffer.Load<DataType>( 0 );
+} 

+ 3 - 0
tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -1101,6 +1101,9 @@ TEST_F(FileTest, ConsumeStructuredBufferGetDimensions) {
 TEST_F(FileTest, ByteAddressBufferLoad) {
   runFileTest("method.byte-address-buffer.load.hlsl");
 }
+TEST_F(FileTest, ByteAddressBufferLoadLayout) {
+  runFileTest("method.byte-address-buffer.load.layout.hlsl");
+}
 TEST_F(FileTest, ByteAddressBufferTemplatedLoadScalar) {
   runFileTest("method.byte-address-buffer.templated-load.scalar.hlsl");
 }