Use Tint SingleEntryPoint transform in Vulkan/GL backends

Some Vulkan drivers don't handle multiple entrypoints well.
In addition, SPIRV-Cross translation can be wrong for
shader modules with multiple entrypoints. Always emit a single
SPIR-V entrypoint to workaround these issues.

This allows updating CopyTextureForBrowser to use a single
shader module, and it fixes some tests with multiple
entrypoints.

Fixed: dawn:948, dawn:959, dawn:1013
Change-Id: Ie129a32a54845316d11917331937ca44fba3d347
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/60640
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/dawn_native/CopyTextureForBrowserHelper.cpp

@@ -33,17 +33,14 @@
 
 namespace dawn_native {
     namespace {
-// TODO(crbug.com/1221110): Remove this header macro by merging vertex and
-// fragment shaders into one shader source. Now it blocks by
-// crbug.com/dawn/947 and crbug.com/tint/915
-#define HEADER                              \
-    "        [[block]] struct Uniforms {\n" \
-    "            u_scale: vec2<f32>;\n"     \
-    "            u_offset: vec2<f32>;\n"    \
-    "            u_alphaOp: u32;\n"         \
-    "        };\n"
 
-        static const char sCopyTextureForBrowserVertex[] = HEADER R"(
+        static const char sCopyTextureForBrowserShader[] = R"(
+            [[block]] struct Uniforms {
+                u_scale: vec2<f32>;
+                u_offset: vec2<f32>;
+                u_alphaOp: u32;
+            };
+
             [[binding(0), group(0)]] var<uniform> uniforms : Uniforms;
 
             struct VertexOutputs {
@@ -51,7 +48,7 @@ namespace dawn_native {
                 [[builtin(position)]] position : vec4<f32>;
             };
 
-            [[stage(vertex)]] fn main(
+            [[stage(vertex)]] fn vs_main(
                 [[builtin(vertex_index)]] VertexIndex : u32
             ) -> VertexOutputs {
                 var texcoord = array<vec2<f32>, 3>(
@@ -84,14 +81,11 @@ namespace dawn_native {
 
                 return output;
             }
-        )";
 
-        static const char sCopyTextureForBrowserFragment[] = HEADER R"(
-            [[binding(0), group(0)]] var<uniform> uniforms : Uniforms;
             [[binding(1), group(0)]] var mySampler: sampler;
             [[binding(2), group(0)]] var myTexture: texture_2d<f32>;
 
-            [[stage(fragment)]] fn main(
+            [[stage(fragment)]] fn fs_main(
                 [[location(0)]] texcoord : vec2<f32>
             ) -> [[location(0)]] vec4<f32> {
                 // Clamp the texcoord and discard the out-of-bound pixels.
@@ -226,39 +220,27 @@ namespace dawn_native {
 
             if (GetCachedPipeline(store, dstFormat) == nullptr) {
                 // Create vertex shader module if not cached before.
-                if (store->copyTextureForBrowserVS == nullptr) {
+                if (store->copyTextureForBrowser == nullptr) {
                     ShaderModuleDescriptor descriptor;
                     ShaderModuleWGSLDescriptor wgslDesc;
-                    wgslDesc.source = sCopyTextureForBrowserVertex;
+                    wgslDesc.source = sCopyTextureForBrowserShader;
                     descriptor.nextInChain = reinterpret_cast<ChainedStruct*>(&wgslDesc);
 
-                    DAWN_TRY_ASSIGN(store->copyTextureForBrowserVS,
+                    DAWN_TRY_ASSIGN(store->copyTextureForBrowser,
                                     device->CreateShaderModule(&descriptor));
                 }
 
-                ShaderModuleBase* vertexModule = store->copyTextureForBrowserVS.Get();
+                ShaderModuleBase* shaderModule = store->copyTextureForBrowser.Get();
-
-                // Create fragment shader module if not cached before.
-                if (store->copyTextureForBrowserFS == nullptr) {
-                    ShaderModuleDescriptor descriptor;
-                    ShaderModuleWGSLDescriptor wgslDesc;
-                    wgslDesc.source = sCopyTextureForBrowserFragment;
-                    descriptor.nextInChain = reinterpret_cast<ChainedStruct*>(&wgslDesc);
-                    DAWN_TRY_ASSIGN(store->copyTextureForBrowserFS,
-                                    device->CreateShaderModule(&descriptor));
-                }
-
-                ShaderModuleBase* fragmentModule = store->copyTextureForBrowserFS.Get();
 
                 // Prepare vertex stage.
                 VertexState vertex = {};
-                vertex.module = vertexModule;
+                vertex.module = shaderModule;
-                vertex.entryPoint = "main";
+                vertex.entryPoint = "vs_main";
 
                 // Prepare frgament stage.
                 FragmentState fragment = {};
-                fragment.module = fragmentModule;
+                fragment.module = shaderModule;
-                fragment.entryPoint = "main";
+                fragment.entryPoint = "fs_main";
 
                 // Prepare color state.
                 ColorTargetState target = {};

src/dawn_native/InternalPipelineStore.h

@@ -28,8 +28,7 @@ namespace dawn_native {
         std::unordered_map<wgpu::TextureFormat, Ref<RenderPipelineBase>>
             copyTextureForBrowserPipelines;
 
-        Ref<ShaderModuleBase> copyTextureForBrowserVS;
+        Ref<ShaderModuleBase> copyTextureForBrowser;
-        Ref<ShaderModuleBase> copyTextureForBrowserFS;
 
         Ref<ComputePipelineBase> timestampComputePipeline;
         Ref<ShaderModuleBase> timestampCS;

src/dawn_native/opengl/ComputePipelineGL.cpp

@@ -18,12 +18,19 @@
 
 namespace dawn_native { namespace opengl {
 
-    ComputePipeline::ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor)
+    // static
-        : ComputePipelineBase(device, descriptor) {
+    ResultOrError<Ref<ComputePipeline>> ComputePipeline::Create(
-        PerStage<const ShaderModule*> modules(nullptr);
+        Device* device,
-        modules[SingleShaderStage::Compute] = ToBackend(descriptor->compute.module);
+        const ComputePipelineDescriptor* descriptor) {
+        Ref<ComputePipeline> pipeline = AcquireRef(new ComputePipeline(device, descriptor));
+        DAWN_TRY(pipeline->Initialize(descriptor));
+        return pipeline;
+    }
 
-        PipelineGL::Initialize(device->gl, ToBackend(descriptor->layout), GetAllStages());
+    MaybeError ComputePipeline::Initialize(const ComputePipelineDescriptor*) {
+        DAWN_TRY(
+            InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages()));
+        return {};
     }
 
     void ComputePipeline::ApplyNow() {

src/dawn_native/opengl/ComputePipelineGL.h

@@ -27,12 +27,16 @@ namespace dawn_native { namespace opengl {
 
     class ComputePipeline final : public ComputePipelineBase, public PipelineGL {
       public:
-        ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor);
+        static ResultOrError<Ref<ComputePipeline>> Create(
+            Device* device,
+            const ComputePipelineDescriptor* descriptor);
 
         void ApplyNow();
 
       private:
+        using ComputePipelineBase::ComputePipelineBase;
         ~ComputePipeline() override = default;
+        MaybeError Initialize(const ComputePipelineDescriptor* descriptor) override;
     };
 
 }}  // namespace dawn_native::opengl

src/dawn_native/opengl/DeviceGL.cpp

@@ -129,7 +129,7 @@ namespace dawn_native { namespace opengl {
     }
     ResultOrError<Ref<ComputePipelineBase>> Device::CreateComputePipelineImpl(
         const ComputePipelineDescriptor* descriptor) {
-        return AcquireRef(new ComputePipeline(this, descriptor));
+        return ComputePipeline::Create(this, descriptor);
     }
     ResultOrError<Ref<PipelineLayoutBase>> Device::CreatePipelineLayoutImpl(
         const PipelineLayoutDescriptor* descriptor) {
@@ -141,7 +141,7 @@ namespace dawn_native { namespace opengl {
     }
     ResultOrError<Ref<RenderPipelineBase>> Device::CreateRenderPipelineImpl(
         const RenderPipelineDescriptor* descriptor) {
-        return AcquireRef(new RenderPipeline(this, descriptor));
+        return RenderPipeline::Create(this, descriptor);
     }
     ResultOrError<Ref<SamplerBase>> Device::CreateSamplerImpl(const SamplerDescriptor* descriptor) {
         return AcquireRef(new Sampler(this, descriptor));

src/dawn_native/opengl/PipelineGL.cpp

@@ -15,7 +15,6 @@
 #include "dawn_native/opengl/PipelineGL.h"
 
 #include "common/BitSetIterator.h"
-#include "common/Log.h"
 #include "dawn_native/BindGroupLayout.h"
 #include "dawn_native/Device.h"
 #include "dawn_native/Pipeline.h"
@@ -26,6 +25,7 @@
 #include "dawn_native/opengl/ShaderModuleGL.h"
 
 #include <set>
+#include <sstream>
 
 namespace dawn_native { namespace opengl {
 
@@ -47,11 +47,11 @@ namespace dawn_native { namespace opengl {
     PipelineGL::PipelineGL() = default;
     PipelineGL::~PipelineGL() = default;
 
-    void PipelineGL::Initialize(const OpenGLFunctions& gl,
+    MaybeError PipelineGL::InitializeBase(const OpenGLFunctions& gl,
                                           const PipelineLayout* layout,
                                           const PerStage<ProgrammableStage>& stages) {
         auto CreateShader = [](const OpenGLFunctions& gl, GLenum type,
-                               const char* source) -> GLuint {
+                               const char* source) -> ResultOrError<GLuint> {
             GLuint shader = gl.CreateShader(type);
             gl.ShaderSource(shader, 1, &source, nullptr);
             gl.CompileShader(shader);
@@ -65,8 +65,9 @@ namespace dawn_native { namespace opengl {
                 if (infoLogLength > 1) {
                     std::vector<char> buffer(infoLogLength);
                     gl.GetShaderInfoLog(shader, infoLogLength, nullptr, &buffer[0]);
-                    dawn::ErrorLog() << source << "\nProgram compilation failed:\n"
+                    std::stringstream ss;
-                                     << buffer.data();
+                    ss << source << "\nProgram compilation failed:\n" << buffer.data();
+                    return DAWN_VALIDATION_ERROR(ss.str().c_str());
                 }
             }
             return shader;
@@ -87,10 +88,12 @@ namespace dawn_native { namespace opengl {
         bool needsDummySampler = false;
         for (SingleShaderStage stage : IterateStages(activeStages)) {
             const ShaderModule* module = ToBackend(stages[stage].module.Get());
-            std::string glsl =
+            std::string glsl;
-                module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage,
+            DAWN_TRY_ASSIGN(glsl, module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage,
-                                        &combinedSamplers[stage], layout, &needsDummySampler);
+                                                          &combinedSamplers[stage], layout,
-            GLuint shader = CreateShader(gl, GLShaderType(stage), glsl.c_str());
+                                                          &needsDummySampler));
+            GLuint shader;
+            DAWN_TRY_ASSIGN(shader, CreateShader(gl, GLShaderType(stage), glsl.c_str()));
             gl.AttachShader(mProgram, shader);
         }
 
@@ -115,7 +118,9 @@ namespace dawn_native { namespace opengl {
             if (infoLogLength > 1) {
                 std::vector<char> buffer(infoLogLength);
                 gl.GetProgramInfoLog(mProgram, infoLogLength, nullptr, &buffer[0]);
-                dawn::ErrorLog() << "Program link failed:\n" << buffer.data();
+                std::stringstream ss;
+                ss << "Program link failed:\n" << buffer.data();
+                return DAWN_VALIDATION_ERROR(ss.str().c_str());
             }
         }
 
@@ -172,6 +177,7 @@ namespace dawn_native { namespace opengl {
 
             textureUnit++;
         }
+        return {};
     }
 
     const std::vector<PipelineGL::SamplerUnit>& PipelineGL::GetTextureUnitsForSampler(

src/dawn_native/opengl/PipelineGL.h

@@ -37,10 +37,6 @@ namespace dawn_native { namespace opengl {
         PipelineGL();
         ~PipelineGL();
 
-        void Initialize(const OpenGLFunctions& gl,
-                        const PipelineLayout* layout,
-                        const PerStage<ProgrammableStage>& stages);
-
         // For each unit a sampler is bound to we need to know if we should use filtering or not
         // because int and uint texture are only complete without filtering.
         struct SamplerUnit {
@@ -53,6 +49,11 @@ namespace dawn_native { namespace opengl {
 
         void ApplyNow(const OpenGLFunctions& gl);
 
+      protected:
+        MaybeError InitializeBase(const OpenGLFunctions& gl,
+                                  const PipelineLayout* layout,
+                                  const PerStage<ProgrammableStage>& stages);
+
       private:
         GLuint mProgram;
         std::vector<std::vector<SamplerUnit>> mUnitsForSamplers;

src/dawn_native/opengl/RenderPipelineGL.cpp

@@ -219,16 +219,26 @@ namespace dawn_native { namespace opengl {
 
     }  // anonymous namespace
 
+    // static
+    ResultOrError<Ref<RenderPipeline>> RenderPipeline::Create(
+        Device* device,
+        const RenderPipelineDescriptor* descriptor) {
+        Ref<RenderPipeline> pipeline = AcquireRef(new RenderPipeline(device, descriptor));
+        DAWN_TRY(pipeline->Initialize());
+        return pipeline;
+    }
+
     RenderPipeline::RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor)
         : RenderPipelineBase(device, descriptor),
           mVertexArrayObject(0),
           mGlPrimitiveTopology(GLPrimitiveTopology(GetPrimitiveTopology())) {
-        PerStage<const ShaderModule*> modules(nullptr);
+    }
-        modules[SingleShaderStage::Vertex] = ToBackend(descriptor->vertex.module);
-        modules[SingleShaderStage::Fragment] = ToBackend(descriptor->fragment->module);
 
-        PipelineGL::Initialize(device->gl, ToBackend(GetLayout()), GetAllStages());
+    MaybeError RenderPipeline::Initialize() {
+        DAWN_TRY(
+            InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages()));
         CreateVAOForVertexState();
+        return {};
     }
 
     RenderPipeline::~RenderPipeline() {

src/dawn_native/opengl/RenderPipelineGL.h

@@ -29,7 +29,9 @@ namespace dawn_native { namespace opengl {
 
     class RenderPipeline final : public RenderPipelineBase, public PipelineGL {
       public:
-        RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor);
+        static ResultOrError<Ref<RenderPipeline>> Create(
+            Device* device,
+            const RenderPipelineDescriptor* descriptor);
 
         GLenum GetGLPrimitiveTopology() const;
         ityp::bitset<VertexAttributeLocation, kMaxVertexAttributes> GetAttributesUsingVertexBuffer(
@@ -38,7 +40,10 @@ namespace dawn_native { namespace opengl {
         void ApplyNow(PersistentPipelineState& persistentPipelineState);
 
       private:
+        RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor);
         ~RenderPipeline() override;
+        MaybeError Initialize();
+
         void CreateVAOForVertexState();
 
         // TODO(yunchao.he@intel.com): vao need to be deduplicated between pipelines.

src/dawn_native/opengl/ShaderModuleGL.cpp

@@ -94,14 +94,14 @@ namespace dawn_native { namespace opengl {
                 return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
             }
 
-            mGLSpirv = std::move(result.spirv);
+            DAWN_TRY_ASSIGN(mGLEntryPoints,
-            DAWN_TRY_ASSIGN(mGLEntryPoints, ReflectShaderUsingSPIRVCross(GetDevice(), mGLSpirv));
+                            ReflectShaderUsingSPIRVCross(GetDevice(), result.spirv));
         }
 
         return {};
     }
 
-    std::string ShaderModule::TranslateToGLSL(const char* entryPointName,
+    ResultOrError<std::string> ShaderModule::TranslateToGLSL(const char* entryPointName,
                                                              SingleShaderStage stage,
                                                              CombinedSamplerInfo* combinedSamplers,
                                                              const PipelineLayout* layout,
@@ -125,8 +125,32 @@ namespace dawn_native { namespace opengl {
         options.es = version.IsES();
         options.version = version.GetMajor() * 100 + version.GetMinor() * 10;
 
-        spirv_cross::CompilerGLSL compiler(
+        std::vector<uint32_t> spirv;
-            GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator) ? mGLSpirv : GetSpirv());
+        if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) {
+            tint::transform::SingleEntryPoint singleEntryPointTransform;
+
+            tint::transform::DataMap transformInputs;
+            transformInputs.Add<tint::transform::SingleEntryPoint::Config>(entryPointName);
+
+            tint::Program program;
+            DAWN_TRY_ASSIGN(program, RunTransforms(&singleEntryPointTransform, GetTintProgram(),
+                                                   transformInputs, nullptr, nullptr));
+
+            tint::writer::spirv::Options options;
+            options.disable_workgroup_init =
+                GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
+            auto result = tint::writer::spirv::Generate(&program, options);
+            if (!result.success) {
+                std::ostringstream errorStream;
+                errorStream << "Generator: " << result.error << std::endl;
+                return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
+            }
+
+            spirv = std::move(result.spirv);
+        } else {
+            spirv = GetSpirv();
+        }
+        spirv_cross::CompilerGLSL compiler(std::move(spirv));
         compiler.set_common_options(options);
         compiler.set_entry_point(entryPointName, ShaderStageToExecutionModel(stage));
 

src/dawn_native/opengl/ShaderModuleGL.h

@@ -50,7 +50,7 @@ namespace dawn_native { namespace opengl {
                                                        const ShaderModuleDescriptor* descriptor,
                                                        ShaderModuleParseResult* parseResult);
 
-        std::string TranslateToGLSL(const char* entryPointName,
+        ResultOrError<std::string> TranslateToGLSL(const char* entryPointName,
                                                    SingleShaderStage stage,
                                                    CombinedSamplerInfo* combinedSamplers,
                                                    const PipelineLayout* layout,
@@ -61,7 +61,6 @@ namespace dawn_native { namespace opengl {
         ~ShaderModule() override = default;
         MaybeError Initialize(ShaderModuleParseResult* parseResult);
 
-        std::vector<uint32_t> mGLSpirv;
         EntryPointMetadataTable mGLEntryPoints;
     };
 

src/dawn_native/vulkan/ShaderModuleVk.cpp

@@ -104,7 +104,6 @@ namespace dawn_native { namespace vulkan {
             DAWN_TRY_ASSIGN(program,
                             RunTransforms(&transformManager, parseResult->tintProgram.get(),
                                           transformInputs, nullptr, nullptr));
-            // We will miss the messages generated in this RunTransforms.
 
             tint::writer::spirv::Options options;
             options.emit_vertex_point_size = true;
@@ -203,11 +202,14 @@ namespace dawn_native { namespace vulkan {
 
         tint::transform::Manager transformManager;
         transformManager.append(std::make_unique<tint::transform::BindingRemapper>());
+        // Many Vulkan drivers can't handle multi-entrypoint shader modules.
+        transformManager.append(std::make_unique<tint::transform::SingleEntryPoint>());
 
         tint::transform::DataMap transformInputs;
         transformInputs.Add<BindingRemapper::Remappings>(std::move(bindingPoints),
                                                          std::move(accessControls),
                                                          /* mayCollide */ false);
+        transformInputs.Add<tint::transform::SingleEntryPoint::Config>(entryPointName);
 
         tint::Program program;
         DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, GetTintProgram(), transformInputs,

src/tests/end2end/CopyTextureForBrowserTests.cpp

@@ -140,6 +140,10 @@ class CopyTextureForBrowserTests : public DawnTest {
 
     void SetUp() override {
         DawnTest::SetUp();
+        // crbug.com/dawn/948: Tint required for multiple entrypoints in a module.
+        // CopyTextureForBrowser uses and internal pipeline with a multi-entrypoint
+        // shader module.
+        DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
 
         testPipeline = MakeTestPipeline();
 

src/tests/end2end/ShaderTests.cpp

@@ -262,8 +262,8 @@ fn main(input : FragmentIn) -> [[location(0)]] vec4<f32> {
 
 // Tests that shaders I/O structs can be shared between vertex and fragment shaders.
 TEST_P(ShaderTests, WGSLSharedStructIO) {
-    // TODO(tint:714): Not yet implemeneted in tint yet, but intended to work.
+    // crbug.com/dawn/948: Tint required for multiple entrypoints in a module.
-    DAWN_SUPPRESS_TEST_IF(IsD3D12() || IsVulkan() || IsMetal() || IsOpenGL() || IsOpenGLES());
+    DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
 
     std::string shader = R"(
 struct VertexIn {