[spirv-writer] Use const for function parameters

The backend was wrongly generating OpLoad instructions for function parameter
accesses since it thought they were pointers.

Run SPIR-V validation for the entry point IO tests.

Bug: tint:509
Change-Id: Ic816a22973fb87c7ede26bad8fb595764a333250
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45941
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: James Price <jrprice@google.com>
This commit is contained in:
James Price 2021-03-25 20:49:37 +00:00 committed by Commit Bot service account
parent ea855a9144
commit a696b027b3
3 changed files with 81 additions and 99 deletions

View File

@ -156,8 +156,7 @@ void Spirv::HandleEntryPointIOTypes(CloneContext& ctx) const {
// Create a function that writes a return value to all output variables. // Create a function that writes a return value to all output variables.
auto* store_value = auto* store_value =
ctx.dst->Var(store_value_symbol, ctx.Clone(func->return_type()), ctx.dst->Const(store_value_symbol, ctx.Clone(func->return_type()));
ast::StorageClass::kFunction, nullptr);
auto return_func_symbol = ctx.dst->Symbols().New(); auto return_func_symbol = ctx.dst->Symbols().New();
auto* return_func = ctx.dst->create<ast::Function>( auto* return_func = ctx.dst->create<ast::Function>(
return_func_symbol, ast::VariableList{store_value}, return_func_symbol, ast::VariableList{store_value},
@ -268,11 +267,8 @@ Symbol Spirv::HoistToInputVariables(
} }
// Create a function-scope variable for the struct. // Create a function-scope variable for the struct.
// TODO(jrprice): Use Const when crbug.com/tint/662 is fixed
auto* initializer = ctx.dst->Construct(ctx.Clone(ty), init_values); auto* initializer = ctx.dst->Construct(ctx.Clone(ty), init_values);
auto* func_var = auto* func_var = ctx.dst->Const(func_var_symbol, ctx.Clone(ty), initializer);
ctx.dst->Var(func_var_symbol, ctx.Clone(ty), ast::StorageClass::kFunction,
initializer, ast::DecorationList{});
ctx.InsertBefore(*func->body()->begin(), ctx.dst->WrapInStatement(func_var)); ctx.InsertBefore(*func->body()->begin(), ctx.dst->WrapInStatement(func_var));
return func_var_symbol; return func_var_symbol;
} }

View File

@ -214,7 +214,7 @@ struct FragmentInput {
[[stage(fragment)]] [[stage(fragment)]]
fn frag_main() -> void { fn frag_main() -> void {
var tint_symbol_6 : FragmentInput = FragmentInput(tint_symbol_4, tint_symbol_5); const tint_symbol_6 : FragmentInput = FragmentInput(tint_symbol_4, tint_symbol_5);
var col : f32 = (tint_symbol_6.coord.x * tint_symbol_6.value); var col : f32 = (tint_symbol_6.coord.x * tint_symbol_6.value);
} }
)"; )";
@ -282,10 +282,10 @@ struct FragmentInput {
[[stage(fragment)]] [[stage(fragment)]]
fn frag_main() -> void { fn frag_main() -> void {
var tint_symbol_13 : Builtins = Builtins(tint_symbol_12); const tint_symbol_13 : Builtins = Builtins(tint_symbol_12);
var tint_symbol_16 : Locations = Locations(tint_symbol_14, tint_symbol_15); const tint_symbol_16 : Locations = Locations(tint_symbol_14, tint_symbol_15);
var tint_symbol_17 : Other = Other(tint_symbol_16); const tint_symbol_17 : Other = Other(tint_symbol_16);
var tint_symbol_19 : FragmentInput = FragmentInput(tint_symbol_13, tint_symbol_17, tint_symbol_18); const tint_symbol_19 : FragmentInput = FragmentInput(tint_symbol_13, tint_symbol_17, tint_symbol_18);
var col : f32 = (tint_symbol_19.b.coord.x * tint_symbol_19.value); var col : f32 = (tint_symbol_19.b.coord.x * tint_symbol_19.value);
var l : f32 = (tint_symbol_19.o.l.l2 + tint_symbol_19.o.l.l3); var l : f32 = (tint_symbol_19.o.l.l2 + tint_symbol_19.o.l.l3);
} }
@ -499,7 +499,7 @@ fn tint_symbol_7(tint_symbol_5 : Interface) -> void {
[[stage(vertex)]] [[stage(vertex)]]
fn vert_main() -> void { fn vert_main() -> void {
var tint_symbol_4 : Interface = Interface(tint_symbol_3); const tint_symbol_4 : Interface = Interface(tint_symbol_3);
tint_symbol_7(tint_symbol_4); tint_symbol_7(tint_symbol_4);
return; return;
} }
@ -548,7 +548,7 @@ fn vert_main() -> void {
[[stage(fragment)]] [[stage(fragment)]]
fn frag_main() -> void { fn frag_main() -> void {
var tint_symbol_8 : Interface = Interface(tint_symbol_7); const tint_symbol_8 : Interface = Interface(tint_symbol_7);
var x : f32 = tint_symbol_8.value; var x : f32 = tint_symbol_8.value;
} }
)"; )";
@ -621,8 +621,8 @@ fn vert_main() -> void {
[[stage(fragment)]] [[stage(fragment)]]
fn frag_main() -> void { fn frag_main() -> void {
var tint_symbol_15 : Interface = Interface(tint_symbol_14); const tint_symbol_15 : Interface = Interface(tint_symbol_14);
var tint_symbol_16 : FragmentInput = FragmentInput(tint_symbol_13, tint_symbol_15); const tint_symbol_16 : FragmentInput = FragmentInput(tint_symbol_13, tint_symbol_15);
var x : f32 = tint_symbol_16.interface.value; var x : f32 = tint_symbol_16.interface.value;
} }
)"; )";
@ -690,8 +690,8 @@ fn tint_symbol_17(tint_symbol_13 : MyVertexOutput) -> void {
[[stage(vertex)]] [[stage(vertex)]]
fn vert_main() -> void { fn vert_main() -> void {
var tint_symbol_9 : MyLocation = MyLocation(tint_symbol_8); const tint_symbol_9 : MyLocation = MyLocation(tint_symbol_8);
var tint_symbol_11 : MyVertexInput = MyVertexInput(tint_symbol_9, tint_symbol_10); const tint_symbol_11 : MyVertexInput = MyVertexInput(tint_symbol_9, tint_symbol_10);
tint_symbol_17(tint_symbol_11); tint_symbol_17(tint_symbol_11);
return; return;
} }
@ -746,7 +746,7 @@ fn tint_symbol_10(tint_symbol_8 : FragmentOutput) -> void {
[[stage(fragment)]] [[stage(fragment)]]
fn frag_main() -> void { fn frag_main() -> void {
var tint_symbol_7 : FragmentInput = FragmentInput(tint_symbol_5, tint_symbol_6); const tint_symbol_7 : FragmentInput = FragmentInput(tint_symbol_5, tint_symbol_6);
tint_symbol_10(FragmentOutput((tint_symbol_7.coord.x * tint_symbol_7.value))); tint_symbol_10(FragmentOutput((tint_symbol_7.coord.x * tint_symbol_7.value)));
return; return;
} }

View File

@ -95,6 +95,8 @@ OpStore %17 %16
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
)"); )");
Validate(b);
} }
TEST_F(BuilderTest, EntryPoint_ReturnValue) { TEST_F(BuilderTest, EntryPoint_ReturnValue) {
@ -129,13 +131,13 @@ TEST_F(BuilderTest, EntryPoint_ReturnValue) {
// Output storage class, and the return statements are replaced with stores. // Output storage class, and the return statements are replaced with stores.
EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader
OpMemoryModel Logical GLSL450 OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %15 "frag_main" %1 %4 OpEntryPoint Fragment %14 "frag_main" %1 %4
OpExecutionMode %15 OriginUpperLeft OpExecutionMode %14 OriginUpperLeft
OpName %1 "tint_symbol_1" OpName %1 "tint_symbol_1"
OpName %4 "tint_symbol_3" OpName %4 "tint_symbol_3"
OpName %10 "tint_symbol_4" OpName %10 "tint_symbol_4"
OpName %11 "tint_symbol_2" OpName %11 "tint_symbol_2"
OpName %15 "frag_main" OpName %14 "frag_main"
OpDecorate %1 Location 0 OpDecorate %1 Location 0
OpDecorate %4 Location 0 OpDecorate %4 Location 0
%3 = OpTypeInt 32 0 %3 = OpTypeInt 32 0
@ -147,32 +149,33 @@ OpDecorate %4 Location 0
%4 = OpVariable %5 Output %7 %4 = OpVariable %5 Output %7
%9 = OpTypeVoid %9 = OpTypeVoid
%8 = OpTypeFunction %9 %6 %8 = OpTypeFunction %9 %6
%14 = OpTypeFunction %9 %13 = OpTypeFunction %9
%18 = OpConstant %3 10 %17 = OpConstant %3 10
%20 = OpTypeBool %19 = OpTypeBool
%24 = OpConstant %6 0.5 %23 = OpConstant %6 0.5
%26 = OpConstant %6 1 %25 = OpConstant %6 1
%10 = OpFunction %9 None %8 %10 = OpFunction %9 None %8
%11 = OpFunctionParameter %6 %11 = OpFunctionParameter %6
%12 = OpLabel %12 = OpLabel
%13 = OpLoad %6 %11 OpStore %4 %11
OpStore %4 %13
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
%15 = OpFunction %9 None %14 %14 = OpFunction %9 None %13
%16 = OpLabel %15 = OpLabel
%17 = OpLoad %3 %1 %16 = OpLoad %3 %1
%19 = OpUGreaterThan %20 %17 %18 %18 = OpUGreaterThan %19 %16 %17
OpSelectionMerge %21 None OpSelectionMerge %20 None
OpBranchConditional %19 %22 %21 OpBranchConditional %18 %21 %20
%22 = OpLabel
%23 = OpFunctionCall %9 %10 %24
OpReturn
%21 = OpLabel %21 = OpLabel
%25 = OpFunctionCall %9 %10 %26 %22 = OpFunctionCall %9 %10 %23
OpReturn
%20 = OpLabel
%24 = OpFunctionCall %9 %10 %25
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
)"); )");
Validate(b);
} }
TEST_F(BuilderTest, EntryPoint_SharedSubStruct) { TEST_F(BuilderTest, EntryPoint_SharedSubStruct) {
@ -250,10 +253,10 @@ TEST_F(BuilderTest, EntryPoint_SharedSubStruct) {
EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader
OpMemoryModel Logical GLSL450 OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %30 "vert_main" %1 %6 OpEntryPoint Vertex %24 "vert_main" %1 %6
OpEntryPoint Fragment %41 "frag_main" %11 %9 %12 OpEntryPoint Fragment %34 "frag_main" %11 %9 %12
OpExecutionMode %41 OriginUpperLeft OpExecutionMode %34 OriginUpperLeft
OpExecutionMode %41 DepthReplacing OpExecutionMode %34 DepthReplacing
OpName %1 "tint_symbol_9" OpName %1 "tint_symbol_9"
OpName %6 "tint_symbol_10" OpName %6 "tint_symbol_10"
OpName %9 "tint_symbol_13" OpName %9 "tint_symbol_13"
@ -266,15 +269,13 @@ OpName %16 "Interface"
OpMemberName %16 0 "value" OpMemberName %16 0 "value"
OpName %17 "tint_symbol_11" OpName %17 "tint_symbol_11"
OpName %18 "tint_symbol_8" OpName %18 "tint_symbol_8"
OpName %30 "vert_main" OpName %24 "vert_main"
OpName %37 "tint_symbol_19" OpName %31 "tint_symbol_19"
OpName %38 "tint_symbol_17" OpName %32 "tint_symbol_17"
OpName %41 "frag_main" OpName %34 "frag_main"
OpName %45 "tint_symbol_15" OpName %38 "FragmentInput"
OpName %48 "FragmentInput" OpMemberName %38 0 "mul"
OpMemberName %48 0 "mul" OpMemberName %38 1 "interface"
OpMemberName %48 1 "interface"
OpName %52 "tint_symbol_16"
OpDecorate %1 BuiltIn Position OpDecorate %1 BuiltIn Position
OpDecorate %6 Location 1 OpDecorate %6 Location 1
OpDecorate %9 Location 0 OpDecorate %9 Location 0
@ -283,8 +284,8 @@ OpDecorate %12 BuiltIn FragDepth
OpMemberDecorate %15 0 Offset 0 OpMemberDecorate %15 0 Offset 0
OpMemberDecorate %15 1 Offset 16 OpMemberDecorate %15 1 Offset 16
OpMemberDecorate %16 0 Offset 0 OpMemberDecorate %16 0 Offset 0
OpMemberDecorate %48 0 Offset 0 OpMemberDecorate %38 0 Offset 0
OpMemberDecorate %48 1 Offset 4 OpMemberDecorate %38 1 Offset 4
%4 = OpTypeFloat 32 %4 = OpTypeFloat 32
%3 = OpTypeVector %4 4 %3 = OpTypeVector %4 4
%2 = OpTypePointer Output %3 %2 = OpTypePointer Output %3
@ -301,64 +302,49 @@ OpMemberDecorate %48 1 Offset 4
%16 = OpTypeStruct %4 %16 = OpTypeStruct %4
%15 = OpTypeStruct %3 %16 %15 = OpTypeStruct %3 %16
%13 = OpTypeFunction %14 %15 %13 = OpTypeFunction %14 %15
%20 = OpTypeInt 32 0 %23 = OpTypeFunction %14
%21 = OpConstant %20 0 %27 = OpConstant %4 42
%22 = OpTypePointer Function %3 %28 = OpConstantComposite %16 %27
%25 = OpConstant %20 1 %29 = OpConstantComposite %15 %5 %28
%26 = OpTypePointer Function %4 %30 = OpTypeFunction %14 %4
%29 = OpTypeFunction %14 %38 = OpTypeStruct %4 %16
%33 = OpConstant %4 42
%34 = OpConstantComposite %16 %33
%35 = OpConstantComposite %15 %5 %34
%36 = OpTypeFunction %14 %4
%46 = OpTypePointer Function %16
%47 = OpConstantNull %16
%48 = OpTypeStruct %4 %16
%53 = OpTypePointer Function %48
%54 = OpConstantNull %48
%17 = OpFunction %14 None %13 %17 = OpFunction %14 None %13
%18 = OpFunctionParameter %15 %18 = OpFunctionParameter %15
%19 = OpLabel %19 = OpLabel
%23 = OpAccessChain %22 %18 %21 %20 = OpCompositeExtract %3 %18 0
%24 = OpLoad %3 %23 OpStore %1 %20
OpStore %1 %24 %21 = OpCompositeExtract %16 %18 1
%27 = OpAccessChain %26 %18 %25 %21 %22 = OpCompositeExtract %4 %21 0
%28 = OpLoad %4 %27 OpStore %6 %22
OpStore %6 %28
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
%30 = OpFunction %14 None %29 %24 = OpFunction %14 None %23
%31 = OpLabel %25 = OpLabel
%32 = OpFunctionCall %14 %17 %35 %26 = OpFunctionCall %14 %17 %29
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
%37 = OpFunction %14 None %36 %31 = OpFunction %14 None %30
%38 = OpFunctionParameter %4 %32 = OpFunctionParameter %4
%39 = OpLabel %33 = OpLabel
%40 = OpLoad %4 %38 OpStore %12 %32
OpStore %12 %40
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
%41 = OpFunction %14 None %29 %34 = OpFunction %14 None %23
%42 = OpLabel %35 = OpLabel
%45 = OpVariable %46 Function %47 %36 = OpLoad %4 %11
%52 = OpVariable %53 Function %54 %37 = OpCompositeConstruct %16 %36
%43 = OpLoad %4 %11 %39 = OpLoad %4 %9
%44 = OpCompositeConstruct %16 %43 %40 = OpCompositeConstruct %38 %39 %37
OpStore %45 %44 %42 = OpCompositeExtract %4 %40 0
%49 = OpLoad %4 %9 %43 = OpCompositeExtract %16 %40 1
%50 = OpLoad %16 %45 %44 = OpCompositeExtract %4 %43 0
%51 = OpCompositeConstruct %48 %49 %50 %45 = OpFMul %4 %42 %44
OpStore %52 %51 %41 = OpFunctionCall %14 %31 %45
%56 = OpAccessChain %26 %52 %21
%57 = OpLoad %4 %56
%58 = OpAccessChain %26 %52 %25 %21
%59 = OpLoad %4 %58
%60 = OpFMul %4 %57 %59
%55 = OpFunctionCall %14 %37 %60
OpReturn OpReturn
OpFunctionEnd OpFunctionEnd
)"); )");
Validate(b);
} }
} // namespace } // namespace