Implement inter-stage variable matching rules - Part I

This patch implements the inter-stage variable matching rules on
the attributes 'location', 'base type' and 'composition type'.

In the next patch we will implement the matching rules on the
'interpoliation type' and 'interpoliation sampling'.

BUG=dawn:802
TEST=dawn_unittests

Change-Id: Ic0a273e01dced301d437add83bad3d0c7d94a133
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58363
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>

src/common/Constants.h

@@ -25,6 +25,7 @@ static constexpr uint32_t kNumStages = 3;
 static constexpr uint8_t kMaxColorAttachments = 8u;
 static constexpr uint32_t kTextureBytesPerRowAlignment = 256u;
 static constexpr uint32_t kMaxInterStageShaderComponents = 60u;
+static constexpr uint32_t kMaxInterStageShaderVariables = kMaxInterStageShaderComponents / 4;
 
 // Compute constants
 static constexpr uint32_t kMaxComputeWorkgroupStorageSize = 16352u;

src/dawn_native/RenderPipeline.cpp

@@ -23,6 +23,7 @@
 #include "dawn_native/VertexFormat.h"
 
 #include <cmath>
+#include <sstream>
 
 namespace dawn_native {
     // Helper functions
@@ -311,6 +312,42 @@ namespace dawn_native {
 
             return {};
         }
+
+        MaybeError ValidateInterStageMatching(DeviceBase* device,
+                                              const VertexState& vertexState,
+                                              const FragmentState& fragmentState) {
+            const EntryPointMetadata& vertexMetadata =
+                vertexState.module->GetEntryPoint(vertexState.entryPoint);
+            const EntryPointMetadata& fragmentMetadata =
+                fragmentState.module->GetEntryPoint(fragmentState.entryPoint);
+
+            if (vertexMetadata.usedInterStageVariables !=
+                fragmentMetadata.usedInterStageVariables) {
+                return DAWN_VALIDATION_ERROR(
+                    "One or more fragment inputs and vertex outputs are not one-to-one matching");
+            }
+
+            auto generateErrorString = [](const char* interStageAttribute, size_t location) {
+                std::ostringstream stream;
+                stream << "The " << interStageAttribute << " of the vertex output at location "
+                       << location
+                       << " is different from the one of the fragment input at the same location";
+                return stream.str();
+            };
+            // TODO(dawn:802): Validate interpolation types and interpolition sampling types
+            for (size_t i : IterateBitSet(vertexMetadata.usedInterStageVariables)) {
+                const auto& vertexOutputInfo = vertexMetadata.interStageVariables[i];
+                const auto& fragmentInputInfo = fragmentMetadata.interStageVariables[i];
+                if (vertexOutputInfo.baseType != fragmentInputInfo.baseType) {
+                    return DAWN_VALIDATION_ERROR(generateErrorString("base type", i));
+                }
+                if (vertexOutputInfo.componentCount != fragmentInputInfo.componentCount) {
+                    return DAWN_VALIDATION_ERROR(generateErrorString("componentCount", i));
+                }
+            }
+
+            return {};
+        }
     }  // anonymous namespace
 
     // Helper functions
@@ -362,6 +399,8 @@ namespace dawn_native {
             return DAWN_VALIDATION_ERROR("Should have at least one color target or a depthStencil");
         }
 
+        DAWN_TRY(ValidateInterStageMatching(device, descriptor->vertex, *(descriptor->fragment)));
+
         return {};
     }
 

src/dawn_native/ShaderModule.cpp

@@ -337,6 +337,38 @@ namespace dawn_native {
             }
         }
 
+        ResultOrError<InterStageComponentType> TintComponentTypeToInterStageComponentType(
+            tint::inspector::ComponentType type) {
+            switch (type) {
+                case tint::inspector::ComponentType::kFloat:
+                    return InterStageComponentType::Float;
+                case tint::inspector::ComponentType::kSInt:
+                    return InterStageComponentType::Sint;
+                case tint::inspector::ComponentType::kUInt:
+                    return InterStageComponentType::Uint;
+                case tint::inspector::ComponentType::kUnknown:
+                    return DAWN_VALIDATION_ERROR(
+                        "Attempted to convert 'Unknown' component type from Tint");
+            }
+        }
+
+        ResultOrError<uint32_t> TintCompositionTypeToInterStageComponentCount(
+            tint::inspector::CompositionType type) {
+            switch (type) {
+                case tint::inspector::CompositionType::kScalar:
+                    return 1u;
+                case tint::inspector::CompositionType::kVec2:
+                    return 2u;
+                case tint::inspector::CompositionType::kVec3:
+                    return 3u;
+                case tint::inspector::CompositionType::kVec4:
+                    return 4u;
+                case tint::inspector::CompositionType::kUnknown:
+                    return DAWN_VALIDATION_ERROR(
+                        "Attempt to convert 'Unknown' composition type from Tint");
+            }
+        }
+
         MaybeError ValidateSpirv(const uint32_t* code, uint32_t codeSize) {
             spvtools::SpirvTools spirvTools(SPV_ENV_VULKAN_1_1);
 
@@ -910,8 +942,7 @@ namespace dawn_native {
                 return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
             }
 
-            constexpr uint32_t kMaxInterStageShaderLocation =
-                kMaxInterStageShaderComponents / 4 - 1;
+            constexpr uint32_t kMaxInterStageShaderLocation = kMaxInterStageShaderVariables - 1;
             for (auto& entryPoint : entryPoints) {
                 ASSERT(result.count(entryPoint.name) == 0);
 
@@ -1006,6 +1037,13 @@ namespace dawn_native {
                             ss << "Vertex output location (" << location << ") over limits";
                             return DAWN_VALIDATION_ERROR(ss.str());
                         }
+                        metadata->usedInterStageVariables.set(location);
+                        DAWN_TRY_ASSIGN(
+                            metadata->interStageVariables[location].baseType,
+                            TintComponentTypeToInterStageComponentType(output_var.component_type));
+                        DAWN_TRY_ASSIGN(metadata->interStageVariables[location].componentCount,
+                                        TintCompositionTypeToInterStageComponentCount(
+                                            output_var.composition_type));
                     }
                 }
 
@@ -1021,6 +1059,13 @@ namespace dawn_native {
                             ss << "Fragment input location (" << location << ") over limits";
                             return DAWN_VALIDATION_ERROR(ss.str());
                         }
+                        metadata->usedInterStageVariables.set(location);
+                        DAWN_TRY_ASSIGN(
+                            metadata->interStageVariables[location].baseType,
+                            TintComponentTypeToInterStageComponentType(input_var.component_type));
+                        DAWN_TRY_ASSIGN(metadata->interStageVariables[location].componentCount,
+                                        TintCompositionTypeToInterStageComponentCount(
+                                            input_var.composition_type));
                     }
 
                     for (const auto& output_var : entryPoint.output_variables) {

src/dawn_native/ShaderModule.h

@@ -53,6 +53,13 @@ namespace dawn_native {
 
     struct EntryPointMetadata;
 
+    // Base component type of an inter-stage variable
+    enum class InterStageComponentType {
+        Sint,
+        Uint,
+        Float,
+    };
+
     using PipelineLayoutEntryPointPair = std::pair<PipelineLayoutBase*, std::string>;
     struct PipelineLayoutEntryPointPairHashFunc {
         size_t operator()(const PipelineLayoutEntryPointPair& pair) const;
@@ -157,6 +164,17 @@ namespace dawn_native {
             fragmentOutputFormatBaseTypes;
         ityp::bitset<ColorAttachmentIndex, kMaxColorAttachments> fragmentOutputsWritten;
 
+        // TODO(dawn:802): store InterpolationType and IntepolationSampling when we add the
+        // validations on them.
+        struct InterStageVariableInfo {
+            InterStageComponentType baseType;
+            uint32_t componentCount;
+        };
+        // Now that we only support vertex and fragment stages, there can't be both inter-stage
+        // inputs and outputs in one shader stage.
+        std::bitset<kMaxInterStageShaderVariables> usedInterStageVariables;
+        std::array<InterStageVariableInfo, kMaxInterStageShaderVariables> interStageVariables;
+
         // The local workgroup size declared for a compute entry point (or 0s otehrwise).
         Origin3D localWorkgroupSize;
 

src/tests/end2end/TextureViewTests.cpp

@@ -498,7 +498,8 @@ class TextureViewRenderingTest : public DawnTest {
         renderPassInfo.cColorAttachments[0].clearColor = {1.0f, 0.0f, 0.0f, 1.0f};
 
         const char* oneColorFragmentShader = R"(
-            [[stage(fragment)]] fn main() -> [[location(0)]] vec4<f32> {
+            [[stage(fragment)]] fn main([[location(0)]] texCoord : vec2<f32>) ->
+                [[location(0)]] vec4<f32> {
                 return vec4<f32>(0.0, 1.0, 0.0, 1.0);
             }
         )";

src/tests/unittests/validation/RenderPipelineValidationTests.cpp

@@ -832,3 +832,124 @@ TEST_F(DepthClampingValidationTest, Success) {
         device.CreateRenderPipeline(&descriptor);
     }
 }
+
+class InterStageVariableMatchingValidationTest : public RenderPipelineValidationTest {
+  protected:
+    void CheckCreatingRenderPipeline(wgpu::ShaderModule vertexModule,
+                                     wgpu::ShaderModule fragmentModule,
+                                     bool shouldSucceed) {
+        utils::ComboRenderPipelineDescriptor descriptor;
+        descriptor.vertex.module = vertexModule;
+        descriptor.cFragment.module = fragmentModule;
+        if (shouldSucceed) {
+            device.CreateRenderPipeline(&descriptor);
+        } else {
+            ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+        }
+    }
+};
+
+// Tests that creating render pipeline should fail when there is a vertex output that doesn't have
+// its corresponding fragment input at the same location, and there is a fragment input that
+// doesn't have its corresponding vertex output at the same location.
+TEST_F(InterStageVariableMatchingValidationTest, MissingDeclarationAtSameLocation) {
+    wgpu::ShaderModule vertexModuleOutputAtLocation0 = utils::CreateShaderModule(device, R"(
+            struct A {
+                [[location(0)]] vout: f32;
+                [[builtin(position)]] pos: vec4<f32>;
+            };
+            [[stage(vertex)]] fn main() -> A {
+                var vertexOut: A;
+                vertexOut.pos = vec4<f32>(0.0, 0.0, 0.0, 1.0);
+                return vertexOut;
+            })");
+    wgpu::ShaderModule fragmentModuleAtLocation0 = utils::CreateShaderModule(device, R"(
+            struct B {
+                [[location(0)]] fin: f32;
+            };
+            [[stage(fragment)]] fn main(fragmentIn: B) -> [[location(0)]] vec4<f32>  {
+                return vec4<f32>(fragmentIn.fin, 0.0, 0.0, 1.0);
+            })");
+    wgpu::ShaderModule fragmentModuleInputAtLocation1 = utils::CreateShaderModule(device, R"(
+            struct A {
+                [[location(1)]] vout: f32;
+            };
+            [[stage(fragment)]] fn main(vertexOut: A) -> [[location(0)]] vec4<f32>  {
+                return vec4<f32>(vertexOut.vout, 0.0, 0.0, 1.0);
+            })");
+    wgpu::ShaderModule vertexModuleOutputAtLocation1 = utils::CreateShaderModule(device, R"(
+            struct B {
+                [[location(1)]] fin: f32;
+                [[builtin(position)]] pos: vec4<f32>;
+            };
+            [[stage(vertex)]] fn main() -> B {
+                var fragmentIn: B;
+                fragmentIn.pos = vec4<f32>(0.0, 0.0, 0.0, 1.0);
+                return fragmentIn;
+            })");
+
+    {
+        CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fsModule, false);
+        CheckCreatingRenderPipeline(vsModule, fragmentModuleAtLocation0, false);
+        CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fragmentModuleInputAtLocation1,
+                                    false);
+        CheckCreatingRenderPipeline(vertexModuleOutputAtLocation1, fragmentModuleAtLocation0,
+                                    false);
+    }
+
+    {
+        CheckCreatingRenderPipeline(vertexModuleOutputAtLocation0, fragmentModuleAtLocation0, true);
+        CheckCreatingRenderPipeline(vertexModuleOutputAtLocation1, fragmentModuleInputAtLocation1,
+                                    true);
+    }
+}
+
+// Tests that creating render pipeline should fail when the type of a vertex stage output variable
+// doesn't match the type of the fragment stage input variable at the same location.
+TEST_F(InterStageVariableMatchingValidationTest, DifferentTypeAtSameLocation) {
+    constexpr std::array<const char*, 12> kTypes = {{"f32", "vec2<f32>", "vec3<f32>", "vec4<f32>",
+                                                     "i32", "vec2<i32>", "vec3<i32>", "vec4<i32>",
+                                                     "u32", "vec2<u32>", "vec3<u32>", "vec4<u32>"}};
+
+    std::array<wgpu::ShaderModule, 12> vertexModules;
+    std::array<wgpu::ShaderModule, 12> fragmentModules;
+    for (uint32_t i = 0; i < kTypes.size(); ++i) {
+        std::string interfaceDeclaration;
+        {
+            std::ostringstream sstream;
+            sstream << "struct A { [[location(0)]] a: " << kTypes[i] << ";" << std::endl;
+            interfaceDeclaration = sstream.str();
+        }
+        {
+            std::ostringstream vertexStream;
+            vertexStream << interfaceDeclaration << R"(
+                    [[builtin(position)]] pos: vec4<f32>;
+                };
+                [[stage(vertex)]] fn main() -> A {
+                    var vertexOut: A;
+                    vertexOut.pos = vec4<f32>(0.0, 0.0, 0.0, 1.0);
+                    return vertexOut;
+                })";
+            vertexModules[i] = utils::CreateShaderModule(device, vertexStream.str().c_str());
+        }
+        {
+            std::ostringstream fragmentStream;
+            fragmentStream << interfaceDeclaration << R"(
+                };
+                [[stage(fragment)]] fn main(fragmentIn: A) -> [[location(0)]] vec4<f32> {
+                    return vec4<f32>(0.0, 0.0, 0.0, 1.0);
+                })";
+            fragmentModules[i] = utils::CreateShaderModule(device, fragmentStream.str().c_str());
+        }
+    }
+
+    for (uint32_t vertexModuleIndex = 0; vertexModuleIndex < kTypes.size(); ++vertexModuleIndex) {
+        wgpu::ShaderModule vertexModule = vertexModules[vertexModuleIndex];
+        for (uint32_t fragmentModuleIndex = 0; fragmentModuleIndex < kTypes.size();
+             ++fragmentModuleIndex) {
+            wgpu::ShaderModule fragmentModule = fragmentModules[fragmentModuleIndex];
+            bool shouldSuccess = vertexModuleIndex == fragmentModuleIndex;
+            CheckCreatingRenderPipeline(vertexModule, fragmentModule, shouldSuccess);
+        }
+    }
+}