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

s-perron · s-perron · commit d486cf4106da · 2025-03-19T09:20:38.000-04:00
This did not solve all of the cases for the issue it was fixing. A new fix was done in the inliner in spirv-opt. This change is no longer needed. This reverts commit 8967dac.
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -3104,12 +3104,6 @@ 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.
@@ -3118,7 +3112,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->getopcode() == spv::Op::OpVariable)) &&
+    if ((argInfo || (argInst && !argInst->isRValue())) &&
         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,14 +28,10 @@ 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: [[ld:%[0-9]+]] = OpLoad %int [[A]]
-// CHECK-NEXT: OpStore [[TempVar]] [[ld]]
-// CHECK-NEXT:     {{%[0-9]+}} = OpFunctionCall %void %foo [[TempVar]] %B %C %D %E
+// CHECK-NEXT:     {{%[0-9]+}} = OpFunctionCall %void %foo [[A]] %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-DAG: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
-// CHECK-DAG: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
+// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
+// CHECK: [[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: [[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]]
+// CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
+// CHECK:                OpStore [[s33]] [[s38]]
 
 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: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
 // CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
+// CHECK: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float 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: [[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]]
+// CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
+// CHECK:                OpStore [[s33]] [[s38]]
 
 [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,13 +11,10 @@ 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-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]]
+
+// CHECK:                OpFunctionCall %void %foo %param_var_a [[p0]] [[p1]]
     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,9 +18,7 @@ float4 main() : C {
 
     float4 val;
 // CHECK:    [[z_ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_float %val %int_2
-// 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:          {{%[0-9]+}} = OpFunctionCall %void %bar %val %param_var_y %param_var_z [[z_ptr]]
 // CHECK-NEXT:   [[y:%[0-9]+]] = OpLoad %v3float %param_var_y
 // CHECK-NEXT: [[old:%[0-9]+]] = OpLoad %v4float %val
     // Write to val.zwx:
@@ -39,10 +37,6 @@ 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,11 +62,7 @@ void main() {
   fn.incr();
 
 // CHECK:      [[rwsb_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_R %rwsb %int_0 %uint_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]]
+// CHECK-NEXT:      {{%[0-9]+}} = OpFunctionCall %void %decr [[rwsb_0]]
   decr(rwsb[0]);
 
 // CHECK: OpFunctionCall %void %decr2 %gs
@@ -91,29 +87,21 @@ void main() {
   fnarr[0].incr();
 
 // CHECK:      [[gsarr_0:%[0-9]+]] = OpAccessChain %_ptr_Workgroup_S %gsarr %int_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]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[gsarr_0]]
   decr2(gsarr[0]);
 
 // CHECK:      [[starr_0:%[0-9]+]] = OpAccessChain %_ptr_Private_S %starr %int_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]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[starr_0]]
   decr2(starr[0]);
 
 // CHECK:      [[fnarr_0:%[0-9]+]] = OpAccessChain %_ptr_Function_S %fnarr %int_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]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %decr2 [[fnarr_0]]
   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:    [[ld:%[0-9]+]] = OpLoad %int [[arr]]
-// CHECK-NEXT:                     OpStore [[TempVar:%[0-9a-zA-Z_]+]] [[ld]]
-// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %int_decr [[TempVar]]
+// CHECK-NEXT:                  OpStore [[arr]] [[arr_1]]
+// CHECK-NEXT:       {{%[0-9]+}} = OpFunctionCall %void %int_decr [[arr]]
   int_decr(++arr[0]);
 }
