GL backend: Make sure remapped entry point is always handled regardless of disable_symbol_renaming

GLSL requires the main enty point to be named "main"
So even when disableSymbolRenaming == false, Tint renames entry point
function named as "main" and other GLSL reserved keywords.
As a result, Dawn needs to retrieve the remapped entry point
in this case.

Bug: dawn:2340
Change-Id: Ifcc9397102650199a6049dd479d3303e069a3af6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/169480
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/opengl/ShaderModuleGL.cpp

diff --git a/src/dawn/native/opengl/ShaderModuleGL.cpp b/src/dawn/native/opengl/ShaderModuleGL.cpp
index 95ee397..fb305d2 100644
--- a/src/dawn/native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn/native/opengl/ShaderModuleGL.cpp
@@ -28,6 +28,7 @@
 #include "dawn/native/opengl/ShaderModuleGL.h"
 
 #include <sstream>
+#include <unordered_map>
 #include <utility>
 
 #include "dawn/native/BindGroupLayoutInternal.h"
@@ -359,20 +360,20 @@
             DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, r.inputProgram,
                                                    transformInputs, &transformOutputs, nullptr));
 
-            // Get the entry point name after the renamer pass.
             // TODO(dawn:2180): refactor out.
+            // Get the entry point name after the renamer pass.
+            // In the case of the entry-point name being a reserved GLSL keyword
+            // (including `main`) the entry-point would have been renamed
+            // regardless of the `disableSymbolRenaming` flag. Always check the
+            // rename map, and if the name was changed, get the new one.
+            auto* data = transformOutputs.Get<tint::ast::transform::Renamer::Data>();
+            DAWN_ASSERT(data != nullptr);
+            auto it = data->remappings.find(r.entryPointName.data());
             std::string remappedEntryPoint;
-            if (r.disableSymbolRenaming) {
-                remappedEntryPoint = r.entryPointName;
-            } else {
-                auto* data = transformOutputs.Get<tint::ast::transform::Renamer::Data>();
-                DAWN_ASSERT(data != nullptr);
-
-                auto it = data->remappings.find(r.entryPointName.data());
-                DAWN_ASSERT(it != data->remappings.end());
+            if (it != data->remappings.end()) {
                 remappedEntryPoint = it->second;
-
-                // Names of inter stage variables need to match
+            } else {
+                remappedEntryPoint = r.entryPointName;
             }
             DAWN_ASSERT(remappedEntryPoint != "");
 

src/dawn/tests/end2end/ShaderTests.cpp

diff --git a/src/dawn/tests/end2end/ShaderTests.cpp b/src/dawn/tests/end2end/ShaderTests.cpp
index b2c4762..b3fd4ff 100644
--- a/src/dawn/tests/end2end/ShaderTests.cpp
+++ b/src/dawn/tests/end2end/ShaderTests.cpp
@@ -2234,6 +2234,76 @@
                                renderPass.color, 32, 16);
 }
 
+// Regression test for crbug.com/dawn/2340. GLSL requires the main enty point to be named "main".
+// We need to make sure when the entry point is "main" or other GLSL reserved keyword,
+// the renaming is always properly handled for the GL backend no matter what
+// "disable_symbol_renaming" is.
+TEST_P(ShaderTests, EntryPointShaderKeywordsComputePipeline) {
+    {
+        // Entry point is "main".
+        std::string shader = R"(
+@compute @workgroup_size(1) fn main() {
+    _ = 1;
+})";
+
+        wgpu::ComputePipeline pipeline = CreateComputePipeline(shader, "main");
+    }
+    {
+        // Entry point is a GLSL reserved keyword other than "main".
+        std::string shader = R"(
+@compute @workgroup_size(1) fn mat2() {
+    _ = 1;
+})";
+
+        wgpu::ComputePipeline pipeline = CreateComputePipeline(shader, "mat2");
+    }
+    {
+        // Entry point is not a GLSL reserved keyword.
+        std::string shader = R"(
+@compute @workgroup_size(1) fn foo1234() {
+    _ = 1;
+})";
+
+        wgpu::ComputePipeline pipeline = CreateComputePipeline(shader, "foo1234");
+    }
+}
+
+// Regression test for crbug.com/dawn/2340. GLSL requires the main enty point to be named "main".
+// We need to make sure when the entry point is "main" or other GLSL reserved keyword,
+// the renaming is always properly handled for the GL backend no matter what
+// "disable_symbol_renaming" is.
+TEST_P(ShaderTests, EntryPointShaderKeywordsRenderPipeline) {
+    std::string shader = R"(
+// Entry point is "main".
+@vertex
+fn main() -> @builtin(position) vec4f {
+    return vec4f(0.0, 0.0, 0.0, 1.0);
+}
+// Entry point is a GLSL reserved keyword other than "main".
+@fragment
+fn mat2() -> @location(0) vec4f {
+    return vec4f(0.0, 0.0, 0.0, 1.0);
+}
+// Entry point is not a GLSL reserved keyword.
+@fragment
+fn foo1234() -> @location(0) vec4f {
+    return vec4f(1.0, 1.0, 1.0, 1.0);
+}
+)";
+    wgpu::ShaderModule shaderModule = utils::CreateShaderModule(device, shader.c_str());
+    utils::ComboRenderPipelineDescriptor rpDesc;
+    rpDesc.vertex.module = shaderModule;
+    rpDesc.cFragment.module = shaderModule;
+    {
+        rpDesc.cFragment.entryPoint = "mat2";
+        wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&rpDesc);
+    }
+    {
+        rpDesc.cFragment.entryPoint = "foo1234";
+        wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&rpDesc);
+    }
+}
+
 TEST_P(ShaderTests, PrivateVarInitWithStruct) {
     wgpu::ComputePipeline pipeline = CreateComputePipeline(R"(
 @binding(0) @group(0) var<storage, read_write> output : i32;
@@ -2367,6 +2437,8 @@
                       MetalBackend(),
                       OpenGLBackend(),
                       OpenGLESBackend(),
+                      OpenGLBackend({"disable_symbol_renaming"}),
+                      OpenGLESBackend({"disable_symbol_renaming"}),
                       VulkanBackend());
 
 }  // anonymous namespace