commit 488d7a9346ff8b63fdba22574ccff1688870e2f3
author dan sinclair <dsinclair@chromium.org> Tue Oct 06 21:20:28 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Oct 06 21:20:28 2020 +0000
tree 4afcc98dd4913e547db0c58673c43f60ca00ffe7
parent 56acec91b15b2f3b42761ab1ef87673bc288f3c7

[spirv-writer] Fix emission of OpExecutionMode

All of the OpEntryPoint declarations must come before OpExecutionMode.
Currently if you have multiple fragment shaders we'll interleave the
OpEntryPoint and OpExeutionMode which will fail to validate.

Bug: tint:263
Change-Id: I7c925cf6b5345c03bfaf1aa15115caa1bdb9af4c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29522
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>

src/writer/spirv/builder_function_decoration_test.cc

diff --git a/src/writer/spirv/builder_function_decoration_test.cc b/src/writer/spirv/builder_function_decoration_test.cc
index 63969cc..26cfcf2 100644
--- a/src/writer/spirv/builder_function_decoration_test.cc
+++ b/src/writer/spirv/builder_function_decoration_test.cc
@@ -48,7 +48,7 @@
   ast::Module mod;
   Builder b(&mod);
   ASSERT_TRUE(b.GenerateFunction(&func)) << b.error();
-  EXPECT_EQ(DumpInstructions(b.preamble()), R"(OpEntryPoint Vertex %3 "main"
+  EXPECT_EQ(DumpInstructions(b.entry_points()), R"(OpEntryPoint Vertex %3 "main"
 )");
 }
 
@@ -73,7 +73,7 @@
   Builder b(&mod);
   ASSERT_TRUE(b.GenerateFunction(&func)) << b.error();
 
-  auto preamble = b.preamble();
+  auto preamble = b.entry_points();
   ASSERT_TRUE(preamble.size() >= 1u);
   EXPECT_EQ(preamble[0].opcode(), spv::Op::OpEntryPoint);
 
@@ -131,7 +131,7 @@
 %10 = OpTypeVoid
 %9 = OpTypeFunction %10
 )");
-  EXPECT_EQ(DumpInstructions(b.preamble()),
+  EXPECT_EQ(DumpInstructions(b.entry_points()),
             R"(OpEntryPoint Vertex %11 "main"
 )");
 }
@@ -200,7 +200,7 @@
 %10 = OpTypeVoid
 %9 = OpTypeFunction %10
 )");
-  EXPECT_EQ(DumpInstructions(b.preamble()),
+  EXPECT_EQ(DumpInstructions(b.entry_points()),
             R"(OpEntryPoint Vertex %11 "main" %4 %1
 )");
 }
@@ -215,7 +215,7 @@
   ast::Module mod;
   Builder b(&mod);
   ASSERT_TRUE(b.GenerateExecutionModes(&func, 3)) << b.error();
-  EXPECT_EQ(DumpInstructions(b.preamble()),
+  EXPECT_EQ(DumpInstructions(b.execution_modes()),
             R"(OpExecutionMode %3 OriginUpperLeft
 )");
 }
@@ -230,7 +230,7 @@
   ast::Module mod;
   Builder b(&mod);
   ASSERT_TRUE(b.GenerateExecutionModes(&func, 3)) << b.error();
-  EXPECT_EQ(DumpInstructions(b.preamble()),
+  EXPECT_EQ(DumpInstructions(b.execution_modes()),
             R"(OpExecutionMode %3 LocalSize 1 1 1
 )");
 }
@@ -246,11 +246,44 @@
   ast::Module mod;
   Builder b(&mod);
   ASSERT_TRUE(b.GenerateExecutionModes(&func, 3)) << b.error();
-  EXPECT_EQ(DumpInstructions(b.preamble()),
+  EXPECT_EQ(DumpInstructions(b.execution_modes()),
             R"(OpExecutionMode %3 LocalSize 2 4 6
 )");
 }
 
+TEST_F(BuilderTest, FunctionDecoration_ExecutionMode_MultipleFragment) {
+  ast::type::VoidType void_type;
+
+  ast::Function func1("main1", {}, &void_type);
+  func1.add_decoration(
+      std::make_unique<ast::StageDecoration>(ast::PipelineStage::kFragment));
+
+  ast::Function func2("main2", {}, &void_type);
+  func2.add_decoration(
+      std::make_unique<ast::StageDecoration>(ast::PipelineStage::kFragment));
+
+  ast::Module mod;
+  Builder b(&mod);
+  ASSERT_TRUE(b.GenerateFunction(&func1)) << b.error();
+  ASSERT_TRUE(b.GenerateFunction(&func2)) << b.error();
+  EXPECT_EQ(DumpBuilder(b),
+            R"(OpEntryPoint Fragment %3 "main1"
+OpEntryPoint Fragment %5 "main2"
+OpExecutionMode %3 OriginUpperLeft
+OpExecutionMode %5 OriginUpperLeft
+OpName %3 "main1"
+OpName %5 "main2"
+%2 = OpTypeVoid
+%1 = OpTypeFunction %2
+%3 = OpFunction %2 None %1
+%4 = OpLabel
+OpFunctionEnd
+%5 = OpFunction %2 None %1
+%6 = OpLabel
+OpFunctionEnd
+)");
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace writer