[SPIRV] Use copy-in/copy-out for non-declaration (microsoft#7127)

s-perron · web-flow · commit 8967dacb03f1 · 2025-02-19T16:26:56.000+01:00
DXC tries to avoid the copy-in-copy-out for some parameters when it thinks it is possible. However, the SPIR-V spec says that if a parameter is a pointer, it must point to a memory object declarion or a pointer to an element in a sampler or image array. This PR enforces this. If the pointer for the parameter is not an OpVariable (a memory object declaration), then we do not elide the copy-in copy-out. Fixes microsoft#7095 Fixes microsoft#5191 Fixes microsoft#3645
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -3104,6 +3104,12 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) {
         argInfo && argInfo->getStorageClass() != spv::StorageClass::Function &&
         isResourceType(paramType);
 
+    // HLSL requires that the parameters be copied in and out from temporaries.
+    // This looks for cases where the copy can be elided. To generate valid
+    // SPIR-V, the argument must be a memory declaration.
+    //
+    //
+
     // If argInfo is nullptr and argInst is a rvalue, we do not have a proper
     // pointer to pass to the function. we need a temporary variable in that
     // case.
@@ -3112,7 +3118,7 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) {
     // create a temporary variable for it because the function definition
     // expects are point-to-pointer argument for resources, which will be
     // resolved by legalization.
-    if ((argInfo || (argInst && !argInst->isRValue())) &&
+    if ((argInfo || (argInst && argInst->getopcode() == spv::Op::OpVariable)) &&
         canActAsOutParmVar(param) && !isArgGlobalVarWithResourceType &&
         paramTypeMatchesArgType(paramType, arg->getType())) {
       // Based on SPIR-V spec, function parameter must be always Function
diff --git a/tools/clang/test/CodeGenSPIRV/cs.groupshared.function-param.out.hlsl b/tools/clang/test/CodeGenSPIRV/cs.groupshared.function-param.out.hlsl
@@ -28,10 +28,14 @@ groupshared S D;
 [numthreads(1,1,1)]
 void main() {
 // CHECK: %E = OpVariable %_ptr_Function_int Function
+// CHECK-NEXT: [[TempVar:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
+
   int E;
 
 // CHECK:        [[A:%[0-9]+]] = OpAccessChain %_ptr_Uniform_int %A %int_0 %uint_0
-// CHECK-NEXT:     {{%[0-9]+}} = OpFunctionCall %void %foo [[A]] %B %C %D %E
+// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %int [[A]]
+// CHECK-NEXT: OpStore [[TempVar]] [[ld]]
+// CHECK-NEXT:     {{%[0-9]+}} = OpFunctionCall %void %foo [[TempVar]] %B %C %D %E
   foo(A[0], B, C, D, E);
   A[0] = A[0] | B | C | D.a | E;
 }
diff --git a/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl
@@ -7,19 +7,19 @@ float4 foo(inout float f0, inout int f1)
     return 0;
 }
 
-// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
-// CHECK: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
+// CHECK-DAG: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
+// CHECK-DAG: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
 // CHECK: [[s33:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Uniform_float {{%[a-zA-Z0-9_]+}} %int_0
-// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int {{%[a-zA-Z0-9_]+}} %int_1
 // CHECK: [[s37:%[a-zA-Z0-9_]+]] = OpLoad %float [[s33]]
 // CHECK:                OpStore [[s36]] [[s37]]
+// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int {{%[a-zA-Z0-9_]+}} %int_1
 // CHECK: [[s40:%[a-zA-Z0-9_]+]] = OpLoad %int [[s34]]
 // CHECK:                OpStore [[s39]] [[s40]]
 // CHECK: {{%[a-zA-Z0-9_]+}} = OpFunctionCall %v4float %foo [[s36]] [[s39]]
-// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
-// CHECK:                OpStore [[s34]] [[s41]]
 // CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
 // CHECK:                OpStore [[s33]] [[s38]]
+// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
+// CHECK:                OpStore [[s34]] [[s41]]
 
 struct Stru {
   int x;
diff --git a/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl b/tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl
@@ -6,19 +6,19 @@ RWStructuredBuffer< float4 > output : register(u1);
 
 // CHECK: OpDecorate %main LinkageAttributes "main" Export
 // CHECK: %main = OpFunction %int None
-// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
 // CHECK: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
+// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
 // CHECK: [[s33:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_StorageBuffer_float {{%[a-zA-Z0-9_]+}} %int_0
-// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int %stru %int_1
 // CHECK: [[s37:%[a-zA-Z0-9_]+]] = OpLoad %float [[s33]]
 // CHECK:                OpStore [[s36]] [[s37]]
+// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int %stru %int_1
 // CHECK: [[s40:%[a-zA-Z0-9_]+]] = OpLoad %int [[s34]]
 // CHECK:                OpStore [[s39]] [[s40]]
 // CHECK: {{%[a-zA-Z0-9_]+}} = OpFunctionCall %void %func [[s36]] [[s39]]
-// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
-// CHECK:                OpStore [[s34]] [[s41]]
 // CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
 // CHECK:                OpStore [[s33]] [[s38]]
+// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
+// CHECK:                OpStore [[s34]] [[s41]]
 
 [noinline]
 void func(inout float f0, inout int f1) {
diff --git a/tools/clang/test/CodeGenSPIRV/fn.param.inout.storage-class.hlsl b/tools/clang/test/CodeGenSPIRV/fn.param.inout.storage-class.hlsl
@@ -11,10 +11,13 @@ void main(float input : INPUT) {
 // CHECK: %param_var_a = OpVariable %_ptr_Function_float Function
 
 // CHECK: [[val:%[0-9]+]] = OpLoad %float %input
-// CHECK:                OpStore %param_var_a [[val]]
+// CHECK:                   OpStore %param_var_a [[val]]
 // CHECK:  [[p0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float %Data %int_0 %uint_0
+// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %float [[p0]]
+// CHECK-NEXT: OpStore [[temp0:%[a-zA-Z0-9_]+]] [[ld]]
 // CHECK:  [[p1:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float %Data %int_0 %uint_1
-
-// CHECK:                OpFunctionCall %void %foo %param_var_a [[p0]] [[p1]]
+// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %float %32
+// CHECK-NEXT: OpStore [[temp1:%[a-zA-Z0-9_]+]] [[ld]]
+// CHECK:                OpFunctionCall %void %foo %param_var_a [[temp0]] [[temp1]]
     foo(input, Data[0], Data[1]);
 }
diff --git a/tools/clang/test/CodeGenSPIRV/fn.param.inout.vector.hlsl b/tools/clang/test/CodeGenSPIRV/fn.param.inout.vector.hlsl
@@ -18,7 +18,9 @@ float4 main() : C {
 
     float4 val;
 // CHECK:    [[z_ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_float %val %int_2
-// CHECK:          {{%[0-9]+}} = OpFunctionCall %void %bar %val %param_var_y %param_var_z [[z_ptr]]
+// CHECK:       [[ld:%[0-9]+]] = OpLoad %float [[z_ptr]]
+// CHECK:                        OpStore %param_var_w [[ld]]
+// CHECK:          {{%[0-9]+}} = OpFunctionCall %void %bar %val %param_var_y %param_var_z %param_var_w
 // CHECK-NEXT:   [[y:%[0-9]+]] = OpLoad %v3float %param_var_y
 // CHECK-NEXT: [[old:%[0-9]+]] = OpLoad %v4float %val
     // Write to val.zwx:
@@ -37,6 +39,10 @@ float4 main() : C {
 // CHECK-NEXT: [[old_0:%[0-9]+]] = OpLoad %v4float %val
 // CHECK-NEXT: [[new_0:%[0-9]+]] = OpVectorShuffle %v4float [[old_0]] [[z]] 4 5 2 3
 // CHECK-NEXT:                OpStore %val [[new_0]]
+    // Write to val.z:
+// CHECK-NEXT: [[new:%[0-9]+]] = OpLoad %float %param_var_w
+// CHECK-NEXT:                   OpStore [[z_ptr]] [[new]]
+
     bar(val, val.zwx, val.xy, val.z);
 
     return MyRWBuffer[0];
diff --git a/tools/clang/test/CodeGenSPIRV/fn.param.isomorphism.hlsl b/tools/clang/test/CodeGenSPIRV/fn.param.isomorphism.hlsl
@@ -62,7 +62,11 @@ void main() {
   fn.incr();
 
 // CHECK:      [[rwsb_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_R %rwsb %int_0 %uint_0
-// CHECK-NEXT:      {{%[0-9]+}} = OpFunctionCall %void %decr [[rwsb_0]]
+// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %R [[rwsb_0]]
+// CHECK-NEXT: [[ex:%[0-9]+]] = OpCompositeExtract %int [[ld]] 0
+// CHECK-NEXT: [[v:%[0-9]+]] = OpCompositeConstruct %R_0 [[ex]]
+// CHECK-NEXT: OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[v]]
+// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr [[TempVar]]
   decr(rwsb[0]);
 
 // CHECK: OpFunctionCall %void %decr2 %gs
@@ -87,21 +91,29 @@ void main() {
   fnarr[0].incr();
 
 // CHECK:      [[gsarr_0:%[0-9]+]] = OpAccessChain %_ptr_Workgroup_S %gsarr %int_0
-// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[gsarr_0]]
+// CHECK:           [[ld:%[0-9]+]] = OpLoad %S [[gsarr_0]]
+// CHECK:                            OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
   decr2(gsarr[0]);
 
 // CHECK:      [[starr_0:%[0-9]+]] = OpAccessChain %_ptr_Private_S %starr %int_0
-// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[starr_0]]
+// CHECK:           [[ld:%[0-9]+]] = OpLoad %S [[starr_0]]
+// CHECK:                            OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
   decr2(starr[0]);
 
 // CHECK:      [[fnarr_0:%[0-9]+]] = OpAccessChain %_ptr_Function_S %fnarr %int_0
-// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[fnarr_0]]
+// CHECK:           [[ld:%[0-9]+]] = OpLoad %S [[fnarr_0]]
+// CHECK:                            OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
   decr2(fnarr[0]);
 
 // CHECK:        [[arr:%[0-9]+]] = OpAccessChain %_ptr_Function_int %arr %int_0
 // CHECK-NEXT: [[arr_0:%[0-9]+]] = OpLoad %int [[arr]]
 // CHECK-NEXT: [[arr_1:%[0-9]+]] = OpIAdd %int [[arr_0]] %int_1
-// CHECK-NEXT:                  OpStore [[arr]] [[arr_1]]
-// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %int_decr [[arr]]
+// CHECK-NEXT:                     OpStore [[arr]] [[arr_1]]
+// CHECK-NEXT:    [[ld:%[0-9]+]] = OpLoad %int [[arr]]
+// CHECK-NEXT:                     OpStore [[TempVar:%[0-9a-zA-Z_]+]] [[ld]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %int_decr [[TempVar]]
   int_decr(++arr[0]);
 }
