Skip to content

Commit 8967dac

Browse files
s-perrons-perron
authored
[SPIRV] Use copy-in/copy-out for non-declaration (microsoft#7127)
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
1 parent fcbd2a1 commit 8967dac

File tree

7 files changed

+52
-21
lines changed

7 files changed

+52
-21
lines changed

‎tools/clang/lib/SPIRV/SpirvEmitter.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3104,6 +3104,12 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) {
31043104
argInfo && argInfo->getStorageClass() != spv::StorageClass::Function &&
31053105
isResourceType(paramType);
31063106

3107+
// HLSL requires that the parameters be copied in and out from temporaries.
3108+
// This looks for cases where the copy can be elided. To generate valid
3109+
// SPIR-V, the argument must be a memory declaration.
3110+
//
3111+
//
3112+
31073113
// If argInfo is nullptr and argInst is a rvalue, we do not have a proper
31083114
// pointer to pass to the function. we need a temporary variable in that
31093115
// case.
@@ -3112,7 +3118,7 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) {
31123118
// create a temporary variable for it because the function definition
31133119
// expects are point-to-pointer argument for resources, which will be
31143120
// resolved by legalization.
3115-
if ((argInfo || (argInst && !argInst->isRValue())) &&
3121+
if ((argInfo || (argInst && argInst->getopcode() == spv::Op::OpVariable)) &&
31163122
canActAsOutParmVar(param) && !isArgGlobalVarWithResourceType &&
31173123
paramTypeMatchesArgType(paramType, arg->getType())) {
31183124
// Based on SPIR-V spec, function parameter must be always Function

‎tools/clang/test/CodeGenSPIRV/cs.groupshared.function-param.out.hlsl‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,14 @@ groupshared S D;
2828
[numthreads(1,1,1)]
2929
void main() {
3030
// CHECK: %E = OpVariable %_ptr_Function_int Function
31+
// CHECK-NEXT: [[TempVar:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
32+
3133
int E;
3234

3335
// CHECK: [[A:%[0-9]+]] = OpAccessChain %_ptr_Uniform_int %A %int_0 %uint_0
34-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %foo [[A]] %B %C %D %E
36+
// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %int [[A]]
37+
// CHECK-NEXT: OpStore [[TempVar]] [[ld]]
38+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %foo [[TempVar]] %B %C %D %E
3539
foo(A[0], B, C, D, E);
3640
A[0] = A[0] | B | C | D.a | E;
3741
}

‎tools/clang/test/CodeGenSPIRV/fn.fixfuncall-compute.hlsl‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ float4 foo(inout float f0, inout int f1)
77
return 0;
88
}
99

10-
// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
11-
// CHECK: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
10+
// CHECK-DAG: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
11+
// CHECK-DAG: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
1212
// CHECK: [[s33:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Uniform_float {{%[a-zA-Z0-9_]+}} %int_0
13-
// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int {{%[a-zA-Z0-9_]+}} %int_1
1413
// CHECK: [[s37:%[a-zA-Z0-9_]+]] = OpLoad %float [[s33]]
1514
// CHECK: OpStore [[s36]] [[s37]]
15+
// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int {{%[a-zA-Z0-9_]+}} %int_1
1616
// CHECK: [[s40:%[a-zA-Z0-9_]+]] = OpLoad %int [[s34]]
1717
// CHECK: OpStore [[s39]] [[s40]]
1818
// CHECK: {{%[a-zA-Z0-9_]+}} = OpFunctionCall %v4float %foo [[s36]] [[s39]]
19-
// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
20-
// CHECK: OpStore [[s34]] [[s41]]
2119
// CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
2220
// CHECK: OpStore [[s33]] [[s38]]
21+
// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
22+
// CHECK: OpStore [[s34]] [[s41]]
2323

2424
struct Stru {
2525
int x;

‎tools/clang/test/CodeGenSPIRV/fn.fixfuncall-linkage.hlsl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,19 @@ RWStructuredBuffer< float4 > output : register(u1);
66

77
// CHECK: OpDecorate %main LinkageAttributes "main" Export
88
// CHECK: %main = OpFunction %int None
9-
// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
109
// CHECK: [[s36:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_float Function
10+
// CHECK: [[s39:%[a-zA-Z0-9_]+]] = OpVariable %_ptr_Function_int Function
1111
// CHECK: [[s33:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_StorageBuffer_float {{%[a-zA-Z0-9_]+}} %int_0
12-
// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int %stru %int_1
1312
// CHECK: [[s37:%[a-zA-Z0-9_]+]] = OpLoad %float [[s33]]
1413
// CHECK: OpStore [[s36]] [[s37]]
14+
// CHECK: [[s34:%[a-zA-Z0-9_]+]] = OpAccessChain %_ptr_Function_int %stru %int_1
1515
// CHECK: [[s40:%[a-zA-Z0-9_]+]] = OpLoad %int [[s34]]
1616
// CHECK: OpStore [[s39]] [[s40]]
1717
// CHECK: {{%[a-zA-Z0-9_]+}} = OpFunctionCall %void %func [[s36]] [[s39]]
18-
// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
19-
// CHECK: OpStore [[s34]] [[s41]]
2018
// CHECK: [[s38:%[a-zA-Z0-9_]+]] = OpLoad %float [[s36]]
2119
// CHECK: OpStore [[s33]] [[s38]]
20+
// CHECK: [[s41:%[a-zA-Z0-9_]+]] = OpLoad %int [[s39]]
21+
// CHECK: OpStore [[s34]] [[s41]]
2222

2323
[noinline]
2424
void func(inout float f0, inout int f1) {

‎tools/clang/test/CodeGenSPIRV/fn.param.inout.storage-class.hlsl‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@ void main(float input : INPUT) {
1111
// CHECK: %param_var_a = OpVariable %_ptr_Function_float Function
1212

1313
// CHECK: [[val:%[0-9]+]] = OpLoad %float %input
14-
// CHECK: OpStore %param_var_a [[val]]
14+
// CHECK: OpStore %param_var_a [[val]]
1515
// CHECK: [[p0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float %Data %int_0 %uint_0
16+
// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %float [[p0]]
17+
// CHECK-NEXT: OpStore [[temp0:%[a-zA-Z0-9_]+]] [[ld]]
1618
// CHECK: [[p1:%[0-9]+]] = OpAccessChain %_ptr_Uniform_float %Data %int_0 %uint_1
17-
18-
// CHECK: OpFunctionCall %void %foo %param_var_a [[p0]] [[p1]]
19+
// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %float %32
20+
// CHECK-NEXT: OpStore [[temp1:%[a-zA-Z0-9_]+]] [[ld]]
21+
// CHECK: OpFunctionCall %void %foo %param_var_a [[temp0]] [[temp1]]
1922
foo(input, Data[0], Data[1]);
2023
}

‎tools/clang/test/CodeGenSPIRV/fn.param.inout.vector.hlsl‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ float4 main() : C {
1818

1919
float4 val;
2020
// CHECK: [[z_ptr:%[0-9]+]] = OpAccessChain %_ptr_Function_float %val %int_2
21-
// CHECK: {{%[0-9]+}} = OpFunctionCall %void %bar %val %param_var_y %param_var_z [[z_ptr]]
21+
// CHECK: [[ld:%[0-9]+]] = OpLoad %float [[z_ptr]]
22+
// CHECK: OpStore %param_var_w [[ld]]
23+
// CHECK: {{%[0-9]+}} = OpFunctionCall %void %bar %val %param_var_y %param_var_z %param_var_w
2224
// CHECK-NEXT: [[y:%[0-9]+]] = OpLoad %v3float %param_var_y
2325
// CHECK-NEXT: [[old:%[0-9]+]] = OpLoad %v4float %val
2426
// Write to val.zwx:
@@ -37,6 +39,10 @@ float4 main() : C {
3739
// CHECK-NEXT: [[old_0:%[0-9]+]] = OpLoad %v4float %val
3840
// CHECK-NEXT: [[new_0:%[0-9]+]] = OpVectorShuffle %v4float [[old_0]] [[z]] 4 5 2 3
3941
// CHECK-NEXT: OpStore %val [[new_0]]
42+
// Write to val.z:
43+
// CHECK-NEXT: [[new:%[0-9]+]] = OpLoad %float %param_var_w
44+
// CHECK-NEXT: OpStore [[z_ptr]] [[new]]
45+
4046
bar(val, val.zwx, val.xy, val.z);
4147

4248
return MyRWBuffer[0];

‎tools/clang/test/CodeGenSPIRV/fn.param.isomorphism.hlsl‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,11 @@ void main() {
6262
fn.incr();
6363

6464
// CHECK: [[rwsb_0:%[0-9]+]] = OpAccessChain %_ptr_Uniform_R %rwsb %int_0 %uint_0
65-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr [[rwsb_0]]
65+
// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %R [[rwsb_0]]
66+
// CHECK-NEXT: [[ex:%[0-9]+]] = OpCompositeExtract %int [[ld]] 0
67+
// CHECK-NEXT: [[v:%[0-9]+]] = OpCompositeConstruct %R_0 [[ex]]
68+
// CHECK-NEXT: OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[v]]
69+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr [[TempVar]]
6670
decr(rwsb[0]);
6771

6872
// CHECK: OpFunctionCall %void %decr2 %gs
@@ -87,21 +91,29 @@ void main() {
8791
fnarr[0].incr();
8892

8993
// CHECK: [[gsarr_0:%[0-9]+]] = OpAccessChain %_ptr_Workgroup_S %gsarr %int_0
90-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[gsarr_0]]
94+
// CHECK: [[ld:%[0-9]+]] = OpLoad %S [[gsarr_0]]
95+
// CHECK: OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
96+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
9197
decr2(gsarr[0]);
9298

9399
// CHECK: [[starr_0:%[0-9]+]] = OpAccessChain %_ptr_Private_S %starr %int_0
94-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[starr_0]]
100+
// CHECK: [[ld:%[0-9]+]] = OpLoad %S [[starr_0]]
101+
// CHECK: OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
102+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
95103
decr2(starr[0]);
96104

97105
// CHECK: [[fnarr_0:%[0-9]+]] = OpAccessChain %_ptr_Function_S %fnarr %int_0
98-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[fnarr_0]]
106+
// CHECK: [[ld:%[0-9]+]] = OpLoad %S [[fnarr_0]]
107+
// CHECK: OpStore [[TempVar:%[a-zA-Z0-9_]+]] [[ld]]
108+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %decr2 [[TempVar]]
99109
decr2(fnarr[0]);
100110

101111
// CHECK: [[arr:%[0-9]+]] = OpAccessChain %_ptr_Function_int %arr %int_0
102112
// CHECK-NEXT: [[arr_0:%[0-9]+]] = OpLoad %int [[arr]]
103113
// CHECK-NEXT: [[arr_1:%[0-9]+]] = OpIAdd %int [[arr_0]] %int_1
104-
// CHECK-NEXT: OpStore [[arr]] [[arr_1]]
105-
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %int_decr [[arr]]
114+
// CHECK-NEXT: OpStore [[arr]] [[arr_1]]
115+
// CHECK-NEXT: [[ld:%[0-9]+]] = OpLoad %int [[arr]]
116+
// CHECK-NEXT: OpStore [[TempVar:%[0-9a-zA-Z_]+]] [[ld]]
117+
// CHECK-NEXT: {{%[0-9]+}} = OpFunctionCall %void %int_decr [[TempVar]]
106118
int_decr(++arr[0]);
107119
}

0 commit comments

Comments
 (0)