[spirv-reader] Only support column-major matrices

- drop ColMajor, MatrixStride member decoration
- RowMajor matrix decoration is an error

Bug: tint:3, tint:99, tint:31
Change-Id: I7eb1ec53813a4b1ada971e8725dc91ffdf97bd43
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/25920
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/reader/spirv/parser_impl.cc

@@ -360,8 +360,20 @@ DecorationList ParserImpl::GetDecorationsForMember(
   return result;
 }
 
+std::string ParserImpl::ShowType(uint32_t type_id) {
+  if (def_use_mgr_) {
+    const auto* type_inst = def_use_mgr_->GetDef(type_id);
+    if (type_inst) {
+      return type_inst->PrettyPrint();
+    }
+  }
+  return "SPIR-V type " + std::to_string(type_id);
+}
+
 std::unique_ptr<ast::StructMemberDecoration>
-ParserImpl::ConvertMemberDecoration(const Decoration& decoration) {
+ParserImpl::ConvertMemberDecoration(uint32_t struct_type_id,
+                                    uint32_t member_index,
+                                    const Decoration& decoration) {
   if (decoration.empty()) {
     Fail() << "malformed SPIR-V decoration: it's empty";
     return nullptr;
@@ -371,7 +383,8 @@ ParserImpl::ConvertMemberDecoration(const Decoration& decoration) {
       if (decoration.size() != 2) {
         Fail()
             << "malformed Offset decoration: expected 1 literal operand, has "
-            << decoration.size() - 1;
+            << decoration.size() - 1 << ": member " << member_index << " of "
+            << ShowType(struct_type_id);
         return nullptr;
       }
       return std::make_unique<ast::StructMemberOffsetDecoration>(decoration[1]);
@@ -380,11 +393,33 @@ ParserImpl::ConvertMemberDecoration(const Decoration& decoration) {
       // TODO(dneto): Drop these for now.
       // https://github.com/gpuweb/gpuweb/issues/935
       return nullptr;
+    case SpvDecorationColMajor:
+      // WGSL only supports column major matrices.
+      return nullptr;
+    case SpvDecorationRowMajor:
+      Fail() << "WGSL does not support row-major matrices: can't "
+                "translate member "
+             << member_index << " of " << ShowType(struct_type_id);
+      return nullptr;
+    case SpvDecorationMatrixStride: {
+      if (decoration.size() != 2) {
+        Fail() << "malformed MatrixStride decoration: expected 1 literal "
+                  "operand, has "
+               << decoration.size() - 1 << ": member " << member_index << " of "
+               << ShowType(struct_type_id);
+        return nullptr;
+      }
+      // TODO(dneto): Fail if the matrix stride is not allocation size of the
+      // column vector of the underlying matrix.  This would need to unpack
+      // any levels of array-ness.
+      return nullptr;
+    }
     default:
       // TODO(dneto): Support the remaining member decorations.
       break;
   }
-  Fail() << "unhandled member decoration: " << decoration[0];
+  Fail() << "unhandled member decoration: " << decoration[0] << " on member "
+         << member_index << " of " << ShowType(struct_type_id);
   return nullptr;
 }
 
@@ -748,7 +783,8 @@ ast::type::Type* ParserImpl::ConvertType(
         Fail() << "unrecognized builtin " << decoration[1];
         return nullptr;
       } else {
-        auto ast_member_decoration = ConvertMemberDecoration(decoration);
+        auto ast_member_decoration =
+            ConvertMemberDecoration(type_id, member_index, decoration);
         if (!success_) {
           return nullptr;
         }

src/reader/spirv/parser_impl.h

@@ -182,14 +182,24 @@ class ParserImpl : Reader {
   DecorationList GetDecorationsForMember(uint32_t id,
                                          uint32_t member_index) const;
 
-  /// Converts a SPIR-V decoration. If the decoration is recognized but
+  /// Converts a SPIR-V struct member decoration. If the decoration is
-  /// deliberately dropped, then returns nullptr without a diagnostic.
+  /// recognized but deliberately dropped, then returns nullptr without a
-  /// On failure, emits a diagnostic and returns nullptr.
+  /// diagnostic. On failure, emits a diagnostic and returns nullptr.
+  /// @param struct_type_id the ID of the struct type
+  /// @param member_index the index of the member
   /// @param decoration an encoded SPIR-V Decoration
   /// @returns the corresponding ast::StructuMemberDecoration
   std::unique_ptr<ast::StructMemberDecoration> ConvertMemberDecoration(
+      uint32_t struct_type_id,
+      uint32_t member_index,
       const Decoration& decoration);
 
+  /// Returns a string for the given type.  If the type ID is invalid,
+  /// then the resulting string only names the type ID.
+  /// @param type_id the SPIR-V ID for the type
+  /// @returns a string description of the type.
+  std::string ShowType(uint32_t type_id);
+
   /// Builds the internal representation of the SPIR-V module.
   /// Assumes the module is somewhat well-formed.  Normally you
   /// would want to validate the SPIR-V module before attempting

src/reader/spirv/parser_impl_convert_member_decoration_test.cc

@@ -34,7 +34,7 @@ using ::testing::Eq;
 TEST_F(SpvParserTest, ConvertMemberDecoration_Empty) {
   auto* p = parser(std::vector<uint32_t>{});
 
-  auto result = p->ConvertMemberDecoration({});
+  auto result = p->ConvertMemberDecoration(1, 1, {});
   EXPECT_EQ(result.get(), nullptr);
   EXPECT_THAT(p->error(), Eq("malformed SPIR-V decoration: it's empty"));
 }
@@ -42,27 +42,25 @@ TEST_F(SpvParserTest, ConvertMemberDecoration_Empty) {
 TEST_F(SpvParserTest, ConvertMemberDecoration_OffsetWithoutOperand) {
   auto* p = parser(std::vector<uint32_t>{});
 
-  auto result = p->ConvertMemberDecoration({SpvDecorationOffset});
+  auto result = p->ConvertMemberDecoration(12, 13, {SpvDecorationOffset});
   EXPECT_EQ(result.get(), nullptr);
-  EXPECT_THAT(
+  EXPECT_THAT(p->error(), Eq("malformed Offset decoration: expected 1 literal "
-      p->error(),
+                             "operand, has 0: member 13 of SPIR-V type 12"));
-      Eq("malformed Offset decoration: expected 1 literal operand, has 0"));
 }
 
 TEST_F(SpvParserTest, ConvertMemberDecoration_OffsetWithTooManyOperands) {
   auto* p = parser(std::vector<uint32_t>{});
 
-  auto result = p->ConvertMemberDecoration({SpvDecorationOffset, 3, 4});
+  auto result = p->ConvertMemberDecoration(12, 13, {SpvDecorationOffset, 3, 4});
   EXPECT_EQ(result.get(), nullptr);
-  EXPECT_THAT(
+  EXPECT_THAT(p->error(), Eq("malformed Offset decoration: expected 1 literal "
-      p->error(),
+                             "operand, has 2: member 13 of SPIR-V type 12"));
-      Eq("malformed Offset decoration: expected 1 literal operand, has 2"));
 }
 
 TEST_F(SpvParserTest, ConvertMemberDecoration_Offset) {
   auto* p = parser(std::vector<uint32_t>{});
 
-  auto result = p->ConvertMemberDecoration({SpvDecorationOffset, 8});
+  auto result = p->ConvertMemberDecoration(1, 1, {SpvDecorationOffset, 8});
   ASSERT_NE(result.get(), nullptr);
   EXPECT_TRUE(result->IsOffset());
   auto* offset_deco = result->AsOffset();
@@ -74,9 +72,10 @@ TEST_F(SpvParserTest, ConvertMemberDecoration_Offset) {
 TEST_F(SpvParserTest, ConvertMemberDecoration_UnhandledDecoration) {
   auto* p = parser(std::vector<uint32_t>{});
 
-  auto result = p->ConvertMemberDecoration({12345678});
+  auto result = p->ConvertMemberDecoration(12, 13, {12345678});
   EXPECT_EQ(result.get(), nullptr);
-  EXPECT_THAT(p->error(), Eq("unhandled member decoration: 12345678"));
+  EXPECT_THAT(p->error(), Eq("unhandled member decoration: 12345678 on member "
+                             "13 of SPIR-V type 12"));
 }
 
 }  // namespace

src/reader/spirv/parser_impl_module_var_test.cc

@@ -1336,6 +1336,78 @@ S -> __struct_S
 })")) << module_str;
 }
 
+TEST_F(SpvParserTest, ModuleScopeVar_ColMajorDecoration_Dropped) {
+  auto* p = parser(test::Assemble(R"(
+     OpName %myvar "myvar"
+     OpDecorate %s Block
+     OpMemberDecorate %s 0 ColMajor
+     %float = OpTypeFloat 32
+     %v2float = OpTypeVector %float 2
+     %m3v2float = OpTypeMatrix %v2float 3
+
+     %s = OpTypeStruct %m3v2float
+     %ptr_sb_s = OpTypePointer StorageBuffer %s
+     %myvar = OpVariable %ptr_sb_s StorageBuffer
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  EXPECT_TRUE(p->error().empty());
+  const auto module_str = p->module().to_str();
+  EXPECT_THAT(module_str, HasSubstr(R"(
+  Variable{
+    myvar
+    storage_buffer
+    __alias_S__struct_S
+  }
+S -> __struct_S
+})")) << module_str;
+}
+
+TEST_F(SpvParserTest, ModuleScopeVar_MatrixStrideDecoration_Dropped) {
+  auto* p = parser(test::Assemble(R"(
+     OpName %myvar "myvar"
+     OpDecorate %s Block
+     OpMemberDecorate %s 0 MatrixStride 8
+     %float = OpTypeFloat 32
+     %v2float = OpTypeVector %float 2
+     %m3v2float = OpTypeMatrix %v2float 3
+
+     %s = OpTypeStruct %m3v2float
+     %ptr_sb_s = OpTypePointer StorageBuffer %s
+     %myvar = OpVariable %ptr_sb_s StorageBuffer
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  EXPECT_TRUE(p->error().empty());
+  const auto module_str = p->module().to_str();
+  EXPECT_THAT(module_str, HasSubstr(R"(
+  Variable{
+    myvar
+    storage_buffer
+    __alias_S__struct_S
+  }
+S -> __struct_S
+})")) << module_str;
+}
+
+TEST_F(SpvParserTest, ModuleScopeVar_RowMajorDecoration_IsError) {
+  auto* p = parser(test::Assemble(R"(
+     OpName %myvar "myvar"
+     OpDecorate %s Block
+     OpMemberDecorate %s 0 RowMajor
+     %float = OpTypeFloat 32
+     %v2float = OpTypeVector %float 2
+     %m3v2float = OpTypeMatrix %v2float 3
+
+     %s = OpTypeStruct %m3v2float
+     %ptr_sb_s = OpTypePointer StorageBuffer %s
+     %myvar = OpVariable %ptr_sb_s StorageBuffer
+  )"));
+  EXPECT_FALSE(p->BuildAndParseInternalModuleExceptFunctions());
+  EXPECT_THAT(
+      p->error(),
+      Eq(R"(WGSL does not support row-major matrices: can't translate member 0 of %2 = OpTypeStruct %5)"))
+      << p->error();
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace reader