Fix the invalid SPIRV decoration issue (#7527)

* Fix the invalid SPIRV decoration issue

Close #7508.

SPIRV doesn't allow decoration on type with Private or Function storage
class.

In our lowering logic, if the array type is used by buffer type it
will always have stride operand after lowering, so if the array is not
used by buffer type, it must be used only by thread_local or
group_shared variable, and it will not have stride operand. For this
case, we don't need to emit stride decoration for SPIRV.

5 files changed, 141 insertions, 8 deletions

diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp
index 3f3b5e707..7bd20649b 100644
--- a/source/slang/slang-emit-spirv.cpp
+++ b/source/slang/slang-emit-spirv.cpp
@@ -2063,14 +2063,21 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
                 if (shouldEmitArrayStride(irArrayType->getElementType()))
                 {
                     auto stride = 0;
+                    // If the array type has no stride, it indicates that this array type is only
+                    // used in Private or Function storage class (aka. thread-local or function
+                    // scope variable), in that case SPIRV doesn't allow to decorate the array
+                    // stride.
+                    //
+                    // The only exception is if the array type is unsized, because unsized array
+                    // also has no stride operand, however unsized array can not be used in Private
+                    // or Function, so we are safe to just decorate the stride for unsized array by
+                    // calculating the natural stride.
                     if (auto strideInst = irArrayType->getArrayStride())
                     {
                         stride = (int)getIntVal(strideInst);
                     }
-                    else
+                    else if (inst->getOp() == kIROp_UnsizedArrayType)
                     {
-                        // Stride may not have been calculated for basic element types. Calculate it
-                        // here.
                         IRSizeAndAlignment sizeAndAlignment;
                         getNaturalSizeAndAlignment(
                             m_targetProgram->getOptionSet(),
@@ -2079,11 +2086,14 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex
                         stride = (int)sizeAndAlignment.getStride();
                     }
 
-                    emitOpDecorateArrayStride(
-                        getSection(SpvLogicalSectionID::Annotations),
-                        nullptr,
-                        arrayType,
-                        SpvLiteralInteger::from32(stride));
+                    if (stride != 0)
+                    {
+                        emitOpDecorateArrayStride(
+                            getSection(SpvLogicalSectionID::Annotations),
+                            nullptr,
+                            arrayType,
+                            SpvLiteralInteger::from32(stride));
+                    }
                 }
                 return arrayType;
             }
diff --git a/tests/spirv/array-stride-decoration-1.slang b/tests/spirv/array-stride-decoration-1.slang
new file mode 100644
index 000000000..5b0f7bc27
--- /dev/null
+++ b/tests/spirv/array-stride-decoration-1.slang
@@ -0,0 +1,29 @@
+//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry computeMain -stage compute -emit-spirv-directly
+
+// The test check that if an array is used in function scope, we don't decorate it with array stride decoration.
+
+// The reason we have to check the SPIRV instead of using spirv-val is because slang uses spv1.6 as the target_env,
+// but the issue only occurs when the target_env is vulkan1.2+
+
+// CHECK: OpEntryPoint
+// CHECK-NOT: OpDecorate %_arr_int_int_2 ArrayStride
+struct S
+{
+    int[2] a;
+}
+
+StructuredBuffer<S> input;
+
+RWStructuredBuffer<int> output;
+
+[shader("compute")]
+void computeMain(uint3 id : SV_DispatchThreadID)
+{
+    S s;
+    s.a = input[0].a;
+
+    for (int i = 0; i < 1; i++)
+    {
+        output[i] = s.a[i];
+    }
+}
diff --git a/tests/spirv/array-stride-decoration-2.slang b/tests/spirv/array-stride-decoration-2.slang
new file mode 100644
index 000000000..702004657
--- /dev/null
+++ b/tests/spirv/array-stride-decoration-2.slang
@@ -0,0 +1,29 @@
+//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry computeMain -stage compute -emit-spirv-directly
+
+// The test check that if an array is used in thread-local, we don't decorate it with array stride decoration.
+
+// The reason we have to check the SPIRV instead of using spirv-val is because slang uses spv1.6 as the target_env,
+// but the issue only occurs when the target_env is vulkan1.2+
+
+// CHECK: OpEntryPoint
+// CHECK-NOT: OpDecorate %_arr_int_int_2 ArrayStride
+struct S
+{
+    int[2] a;
+}
+
+ParameterBlock<S> input;
+
+static S s;
+
+RWStructuredBuffer<int> output;
+
+[shader("compute")]
+void computeMain(uint3 id : SV_DispatchThreadID)
+{
+    s.a = input.a;
+    for (int i = 0; i < 1; i++)
+    {
+        output[i] = s.a[i];
+    }
+}
diff --git a/tests/spirv/array-stride-decoration-3.slang b/tests/spirv/array-stride-decoration-3.slang
new file mode 100644
index 000000000..79a6ef777
--- /dev/null
+++ b/tests/spirv/array-stride-decoration-3.slang
@@ -0,0 +1,31 @@
+//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry computeMain -stage compute -emit-spirv-directly
+
+// The test check that if an array is used in function parameter, we don't decorate it with array stride decoration.
+
+// The reason we have to check the SPIRV instead of using spirv-val is because slang uses spv1.6 as the target_env,
+// but the issue only occurs when the target_env is vulkan1.2+
+
+// CHECK: OpEntryPoint
+// CHECK-NOT: OpDecorate %_arr_int_int_2_0 ArrayStride
+struct S
+{
+    int[2] a;
+}
+
+ParameterBlock<S> input;
+
+RWStructuredBuffer<int> output;
+
+void myfunc(int[2] a)
+{
+    for (int i = 0; i < 2; i++)
+    {
+        output[i] = a[i];
+    }
+}
+
+[shader("compute")]
+void computeMain(uint3 id : SV_DispatchThreadID)
+{
+    myfunc(input.a);
+}
diff --git a/tests/spirv/array-stride-decoration-4.slang b/tests/spirv/array-stride-decoration-4.slang
new file mode 100644
index 000000000..34de071ad
--- /dev/null
+++ b/tests/spirv/array-stride-decoration-4.slang
@@ -0,0 +1,34 @@
+//TEST:SIMPLE(filecheck=CHECK):-target spirv -entry computeMain -stage compute -emit-spirv-directly
+
+// The test check that if an array is used in workgroup, we don't decorate it with array stride decoration.
+
+// The reason we have to check the SPIRV instead of using spirv-val is because slang uses spv1.6 as the target_env,
+// but the issue only occurs when the target_env is vulkan1.2+
+
+// CHECK: OpEntryPoint
+// CHECK-NOT: OpDecorate %_arr_int_int_2 ArrayStride
+struct S
+{
+    int[2] a;
+}
+
+ParameterBlock<S> input;
+
+RWStructuredBuffer<int> output;
+
+groupshared S s;
+
+
+[shader("compute")]
+void computeMain(uint3 id : SV_DispatchThreadID)
+{
+    if (id.x == 0)
+    {
+        for (int i = 0; i < 2; ++i)
+        {
+            s.a[i] = input.a[i];
+        }
+    }
+    GroupMemoryBarrier();
+    output[id.x] = s.a[id.x];
+}