spirv-reader: Fix mixed-signedness binary ops

Fixed: tint:666
Change-Id: I77331081fc5acc128be81e7b85a253bf6ebab608
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46140
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/reader/spirv/function.cc

@@ -3169,7 +3169,8 @@ TypedExpression FunctionEmitter::MaybeEmitCombinatorialValue(
   auto binary_op = ConvertBinaryOp(opcode);
   if (binary_op != ast::BinaryOp::kNone) {
     auto arg0 = MakeOperand(inst, 0);
-    auto arg1 = MakeOperand(inst, 1);
+    auto arg1 = parser_impl_.RectifySecondOperandSignedness(
+        inst, arg0.type, MakeOperand(inst, 1));
     auto* binary_expr = create<ast::BinaryExpression>(Source{}, binary_op,
                                                       arg0.expr, arg1.expr);
     TypedExpression result{ast_type, binary_expr};

src/reader/spirv/function_arithmetic_test.cc

@@ -104,6 +104,15 @@ std::string AstFor(std::string assembly) {
           }
         })";
   }
+  if (assembly == "cast_uint_v2int_40_30") {
+    return R"(Bitcast[not set]<__vec_2__u32>{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{40}
+            ScalarConstructor[not set]{30}
+          }
+        })";
+  }
   if (assembly == "v2float_50_60") {
     return R"(TypeConstructor[not set]{
           __vec_2__f32
@@ -474,8 +483,51 @@ TEST_P(SpvBinaryArithTest, EmitExpression) {
      << GetParam().ast_type << "\n    {\n      Binary[not set]{"
      << "\n        " << GetParam().ast_lhs << "\n        " << GetParam().ast_op
      << "\n        " << GetParam().ast_rhs;
-  EXPECT_THAT(ToString(p->builder(), fe.ast_body()), HasSubstr(ss.str()))
+  auto got = ToString(p->builder(), fe.ast_body());
+  EXPECT_THAT(got, HasSubstr(ss.str())) << "got:\n" << got << assembly;
+}
+
+// Use this when the result might have extra bitcasts on the outside.
+struct BinaryDataGeneral {
+  const std::string res_type;
+  const std::string lhs;
+  const std::string op;
+  const std::string rhs;
+  const std::string expected;
+};
+inline std::ostream& operator<<(std::ostream& out, BinaryDataGeneral data) {
+  out << "BinaryDataGeneral{" << data.res_type << "," << data.lhs << ","
+      << data.op << "," << data.rhs << "," << data.expected << "}";
+  return out;
+}
+
+using SpvBinaryArithGeneralTest =
+    SpvParserTestBase<::testing::TestWithParam<BinaryDataGeneral>>;
+
+TEST_P(SpvBinaryArithGeneralTest, EmitExpression) {
+  const auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %1 = )" + GetParam().op +
+                        " %" + GetParam().res_type + " %" + GetParam().lhs +
+                        " %" + GetParam().rhs + R"(
+     OpReturn
+     OpFunctionEnd
+  )";
+  auto p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions())
+      << p->error() << "\n"
       << assembly;
+  FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
+  EXPECT_TRUE(fe.EmitBody()) << p->error();
+  std::ostringstream ss;
+  ss << R"(VariableConst{
+    x_1
+    none
+    )"
+     << GetParam().expected;
+  auto got = ToString(p->builder(), fe.ast_body());
+  EXPECT_THAT(got, HasSubstr(ss.str())) << "got:\n" << got << assembly;
 }
 
 INSTANTIATE_TEST_SUITE_P(
@@ -490,14 +542,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpIAdd", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "add",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpIAdd", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "add",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpIAdd", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "add",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpIAdd", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "add",
@@ -505,15 +549,108 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpIAdd", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "add",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_IAdd_MixedSignedness,
+    SpvBinaryArithGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpIAdd", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          add
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpIAdd", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        add
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpIAdd", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        add
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpIAdd", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          add
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpIAdd", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpIAdd", "v2uint_10_20",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "add",
+                          R"(__vec_2__u32
-                   AstFor("v2uint_10_20")},
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          add
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpIAdd", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpIAdd", "v2int_40_30",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "add",
+                          R"(__vec_2__i32
-                   AstFor("v2uint_20_10")}));
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          add
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_FAdd,
@@ -540,14 +677,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpISub", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "subtract",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpISub", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "subtract",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpISub", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "subtract",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpISub", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "subtract",
@@ -555,15 +684,108 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpISub", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "subtract",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_ISub_MixedSignedness,
+    SpvBinaryArithGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpISub", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          subtract
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpISub", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        subtract
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpISub", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        subtract
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpISub", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          subtract
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpISub", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpISub", "v2uint_10_20",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "subtract",
+                          R"(__vec_2__u32
-                   AstFor("v2uint_10_20")},
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          subtract
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpISub", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpISub", "v2int_40_30",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "subtract",
+                          R"(__vec_2__i32
-                   AstFor("v2uint_20_10")}));
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          subtract
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_FSub,
@@ -590,14 +812,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpIMul", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "multiply",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpIMul", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "multiply",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpIMul", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "multiply",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpIMul", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "multiply",
@@ -605,15 +819,108 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpIMul", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "multiply",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_IMul_MixedSignedness,
+    SpvBinaryArithGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpIMul", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          multiply
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpIMul", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        multiply
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpIMul", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        multiply
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpIMul", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          multiply
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpIMul", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpIMul", "v2uint_10_20",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "multiply",
+                          R"(__vec_2__u32
-                   AstFor("v2uint_10_20")},
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          multiply
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpIMul", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpIMul", "v2int_40_30",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "multiply",
+                          R"(__vec_2__i32
-                   AstFor("v2uint_20_10")}));
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          multiply
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_FMul,
@@ -1203,7 +1510,7 @@ TEST_F(SpvBinaryArithTestBasic, OuterProduct) {
 
 // TODO(dneto): OpIAddCarry
 // TODO(dneto): OpISubBorrow
-// TODO(dneto): OpIMulExtended
+// TODO(dneto): OpUMulExtended
 // TODO(dneto): OpSMulExtended
 
 }  // namespace

src/reader/spirv/function_bit_test.cc

@@ -163,6 +163,49 @@ TEST_P(SpvBinaryBitTest, EmitExpression) {
       << assembly;
 }
 
+// Use this when the result might have extra bitcasts on the outside.
+struct BinaryDataGeneral {
+  const std::string res_type;
+  const std::string lhs;
+  const std::string op;
+  const std::string rhs;
+  const std::string expected;
+};
+inline std::ostream& operator<<(std::ostream& out, BinaryDataGeneral data) {
+  out << "BinaryDataGeneral{" << data.res_type << "," << data.lhs << ","
+      << data.op << "," << data.rhs << "," << data.expected << "}";
+  return out;
+}
+
+using SpvBinaryBitGeneralTest =
+    SpvParserTestBase<::testing::TestWithParam<BinaryDataGeneral>>;
+
+TEST_P(SpvBinaryBitGeneralTest, EmitExpression) {
+  const auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+     %entry = OpLabel
+     %1 = )" + GetParam().op +
+                        " %" + GetParam().res_type + " %" + GetParam().lhs +
+                        " %" + GetParam().rhs + R"(
+     OpReturn
+     OpFunctionEnd
+  )";
+  auto p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions())
+      << p->error() << "\n"
+      << assembly;
+  FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
+  EXPECT_TRUE(fe.EmitBody()) << p->error();
+  std::ostringstream ss;
+  ss << R"(VariableConst{
+    x_1
+    none
+    )"
+     << GetParam().expected;
+  auto got = ToString(p->builder(), fe.ast_body());
+  EXPECT_THAT(got, HasSubstr(ss.str())) << "got:\n" << got << assembly;
+}
+
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_ShiftLeftLogical,
     SpvBinaryBitTest,
@@ -286,14 +329,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpBitwiseAnd", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "and",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpBitwiseAnd", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "and",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpBitwiseAnd", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "and",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpBitwiseAnd", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "and",
@@ -301,15 +336,110 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpBitwiseAnd", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "and",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_BitwiseAnd_MixedSignedness,
+    SpvBinaryBitGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpBitwiseAnd", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          and
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpBitwiseAnd", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        and
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpBitwiseAnd", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        and
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpBitwiseAnd", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          and
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpBitwiseAnd", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpBitwiseAnd",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "and",
+                          "v2uint_10_20",
-                   AstFor("v2uint_10_20")},
+                          R"(__vec_2__u32
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          and
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpBitwiseAnd", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpBitwiseAnd",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "and",
+                          "v2int_40_30",
-                   AstFor("v2uint_20_10")}));
+                          R"(__vec_2__i32
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          and
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_BitwiseOr,
@@ -323,14 +453,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpBitwiseOr", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "or",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpBitwiseOr", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "or",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpBitwiseOr", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "or",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpBitwiseOr", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "or",
@@ -338,15 +460,109 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpBitwiseOr", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "or",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_BitwiseOr_MixedSignedness,
+    SpvBinaryBitGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpBitwiseOr", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          or
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpBitwiseOr", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        or
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpBitwiseOr", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        or
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpBitwiseOr", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          or
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpBitwiseOr", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpBitwiseOr",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "or",
+                          "v2uint_10_20",
-                   AstFor("v2uint_10_20")},
+                          R"(__vec_2__u32
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          or
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpBitwiseOr", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpBitwiseOr", "v2int_40_30",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "or",
+                          R"(__vec_2__i32
-                   AstFor("v2uint_20_10")}));
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          or
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_BitwiseXor,
@@ -360,14 +576,6 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"int", "int_30", "OpBitwiseXor", "int_40", "__i32",
                    "ScalarConstructor[not set]{30}", "xor",
                    "ScalarConstructor[not set]{40}"},
-        // Mixed, returning uint
-        BinaryData{"uint", "int_30", "OpBitwiseXor", "uint_10", "__u32",
-                   "ScalarConstructor[not set]{30}", "xor",
-                   "ScalarConstructor[not set]{10}"},
-        // Mixed, returning int
-        BinaryData{"int", "int_30", "OpBitwiseXor", "uint_10", "__i32",
-                   "ScalarConstructor[not set]{30}", "xor",
-                   "ScalarConstructor[not set]{10}"},
         // Both v2uint
         BinaryData{"v2uint", "v2uint_10_20", "OpBitwiseXor", "v2uint_20_10",
                    "__vec_2__u32", AstFor("v2uint_10_20"), "xor",
@@ -375,15 +583,110 @@ INSTANTIATE_TEST_SUITE_P(
         // Both v2int
         BinaryData{"v2int", "v2int_30_40", "OpBitwiseXor", "v2int_40_30",
                    "__vec_2__i32", AstFor("v2int_30_40"), "xor",
-                   AstFor("v2int_40_30")},
+                   AstFor("v2int_40_30")}));
+
+INSTANTIATE_TEST_SUITE_P(
+    SpvParserTest_BitwiseXor_MixedSignedness,
+    SpvBinaryBitGeneralTest,
+    ::testing::Values(
+        // Mixed, uint <- int uint
+        BinaryDataGeneral{"uint", "int_30", "OpBitwiseXor", "uint_10",
+                          R"(__u32
+    {
+      Bitcast[not set]<__u32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{30}
+          xor
+          Bitcast[not set]<__i32>{
+            ScalarConstructor[not set]{10}
+          }
+        }
+      }
+    })"},
+        // Mixed, int <- int uint
+        BinaryDataGeneral{"int", "int_30", "OpBitwiseXor", "uint_10",
+                          R"(__i32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{30}
+        xor
+        Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
+        // Mixed, uint <- uint int
+        BinaryDataGeneral{"uint", "uint_10", "OpBitwiseXor", "int_30",
+                          R"(__u32
+    {
+      Binary[not set]{
+        ScalarConstructor[not set]{10}
+        xor
+        Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{30}
+        }
+      }
+    })"},
+        // Mixed, int <- uint uint
+        BinaryDataGeneral{"int", "uint_20", "OpBitwiseXor", "uint_10",
+                          R"(__i32
+    {
+      Bitcast[not set]<__i32>{
+        Binary[not set]{
+          ScalarConstructor[not set]{20}
+          xor
+          ScalarConstructor[not set]{10}
+        }
+      }
+    })"},
         // Mixed, returning v2uint
-        BinaryData{"v2uint", "v2int_30_40", "OpBitwiseXor", "v2uint_10_20",
+        BinaryDataGeneral{"v2uint", "v2int_30_40", "OpBitwiseXor",
-                   "__vec_2__u32", AstFor("v2int_30_40"), "xor",
+                          "v2uint_10_20",
-                   AstFor("v2uint_10_20")},
+                          R"(__vec_2__u32
+    {
+      Bitcast[not set]<__vec_2__u32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__i32
+            ScalarConstructor[not set]{30}
+            ScalarConstructor[not set]{40}
+          }
+          xor
+          Bitcast[not set]<__vec_2__i32>{
+            TypeConstructor[not set]{
+              __vec_2__u32
+              ScalarConstructor[not set]{10}
+              ScalarConstructor[not set]{20}
+            }
+          }
+        }
+      }
+    })"},
         // Mixed, returning v2int
-        BinaryData{"v2int", "v2int_40_30", "OpBitwiseXor", "v2uint_20_10",
+        BinaryDataGeneral{"v2int", "v2uint_10_20", "OpBitwiseXor",
-                   "__vec_2__i32", AstFor("v2int_40_30"), "xor",
+                          "v2int_40_30",
-                   AstFor("v2uint_20_10")}));
+                          R"(__vec_2__i32
+    {
+      Bitcast[not set]<__vec_2__i32>{
+        Binary[not set]{
+          TypeConstructor[not set]{
+            __vec_2__u32
+            ScalarConstructor[not set]{10}
+            ScalarConstructor[not set]{20}
+          }
+          xor
+          Bitcast[not set]<__vec_2__u32>{
+            TypeConstructor[not set]{
+              __vec_2__i32
+              ScalarConstructor[not set]{40}
+              ScalarConstructor[not set]{30}
+            }
+          }
+        }
+      }
+    })"}
+
+        ));
 
 TEST_F(SpvUnaryBitTest, Not_Int_Int) {
   const auto assembly = CommonTypes() + R"(

src/reader/spirv/function_logical_test.cc

@@ -300,19 +300,31 @@ INSTANTIATE_TEST_SUITE_P(
     SpvParserTest_IEqual,
     SpvBinaryLogicalTest,
     ::testing::Values(
-        // Both uint
+        // uint uint
         BinaryData{"bool", "uint_10", "OpIEqual", "uint_20", "__bool",
                    "ScalarConstructor[not set]{10}", "equal",
                    "ScalarConstructor[not set]{20}"},
-        // Both int
+        // int int
         BinaryData{"bool", "int_30", "OpIEqual", "int_40", "__bool",
                    "ScalarConstructor[not set]{30}", "equal",
                    "ScalarConstructor[not set]{40}"},
-        // Both v2uint
+        // uint int
+        BinaryData{"bool", "uint_10", "OpIEqual", "int_40", "__bool",
+                   "ScalarConstructor[not set]{10}", "equal",
+                   R"(Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{40}
+        })"},
+        // int uint
+        BinaryData{"bool", "int_40", "OpIEqual", "uint_10", "__bool",
+                   "ScalarConstructor[not set]{40}", "equal",
+                   R"(Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        })"},
+        // v2uint v2uint
         BinaryData{"v2bool", "v2uint_10_20", "OpIEqual", "v2uint_20_10",
                    "__vec_2__bool", AstFor("v2uint_10_20"), "equal",
                    AstFor("v2uint_20_10")},
-        // Both v2int
+        // v2int v2int
         BinaryData{"v2bool", "v2int_30_40", "OpIEqual", "v2int_40_30",
                    "__vec_2__bool", AstFor("v2int_30_40"), "equal",
                    AstFor("v2int_40_30")}));
@@ -340,6 +352,18 @@ INSTANTIATE_TEST_SUITE_P(
         BinaryData{"bool", "int_30", "OpINotEqual", "int_40", "__bool",
                    "ScalarConstructor[not set]{30}", "not_equal",
                    "ScalarConstructor[not set]{40}"},
+        // uint int
+        BinaryData{"bool", "uint_10", "OpINotEqual", "int_40", "__bool",
+                   "ScalarConstructor[not set]{10}", "not_equal",
+                   R"(Bitcast[not set]<__u32>{
+          ScalarConstructor[not set]{40}
+        })"},
+        // int uint
+        BinaryData{"bool", "int_40", "OpINotEqual", "uint_10", "__bool",
+                   "ScalarConstructor[not set]{40}", "not_equal",
+                   R"(Bitcast[not set]<__i32>{
+          ScalarConstructor[not set]{10}
+        })"},
         // Both v2uint
         BinaryData{"v2bool", "v2uint_10_20", "OpINotEqual", "v2uint_20_10",
                    "__vec_2__bool", AstFor("v2uint_10_20"), "not_equal",

src/reader/spirv/parser_impl.cc

@@ -155,6 +155,29 @@ bool AssumesUnsignedOperands(GLSLstd450 extended_opcode) {
   return false;
 }
 
+// Returns true if the corresponding WGSL operation requires
+// the signedness of the second operand to match the signedness of the
+// first operand, and it's not one of the OpU* or OpS* instructions.
+// (Those are handled via MakeOperand.)
+bool AssumesSecondOperandSignednessMatchesFirstOperand(SpvOp opcode) {
+  switch (opcode) {
+    // All the OpI* integer binary operations.
+    case SpvOpIAdd:
+    case SpvOpISub:
+    case SpvOpIMul:
+    case SpvOpIEqual:
+    case SpvOpINotEqual:
+    // All the bitwise integer binary operations.
+    case SpvOpBitwiseAnd:
+    case SpvOpBitwiseOr:
+    case SpvOpBitwiseXor:
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
 // Returns true if the corresponding WGSL operation requires
 // the signedness of the result to match the signedness of the first operand.
 bool AssumesResultSignednessMatchesFirstOperand(SpvOp opcode) {
@@ -166,6 +189,12 @@ bool AssumesResultSignednessMatchesFirstOperand(SpvOp opcode) {
     case SpvOpSDiv:
     case SpvOpSMod:
     case SpvOpSRem:
+    case SpvOpIAdd:
+    case SpvOpISub:
+    case SpvOpIMul:
+    case SpvOpBitwiseAnd:
+    case SpvOpBitwiseOr:
+    case SpvOpBitwiseXor:
       return true;
     default:
       break;
@@ -1536,6 +1565,21 @@ TypedExpression ParserImpl::RectifyOperandSignedness(
   return std::move(expr);
 }
 
+TypedExpression ParserImpl::RectifySecondOperandSignedness(
+    const spvtools::opt::Instruction& inst,
+    type::Type* first_operand_type,
+    TypedExpression&& second_operand_expr) {
+  if ((first_operand_type != second_operand_expr.type) &&
+      AssumesSecondOperandSignednessMatchesFirstOperand(inst.opcode())) {
+    // Conversion is required.
+    return {first_operand_type,
+            create<ast::BitcastExpression>(Source{}, first_operand_type,
+                                           second_operand_expr.expr)};
+  }
+  // No conversion necessary.
+  return std::move(second_operand_expr);
+}
+
 type::Type* ParserImpl::ForcedResultType(const spvtools::opt::Instruction& inst,
                                          type::Type* first_operand_type) {
   const auto opcode = inst.opcode();

src/reader/spirv/parser_impl.h

@@ -327,6 +327,19 @@ class ParserImpl : Reader {
       const spvtools::opt::Instruction& inst,
       TypedExpression&& expr);
 
+  /// Conversts a second operand to the signedness of the first operand
+  /// of a binary operator, if the WGSL operator requires they be the same.
+  /// Returns the converted expression, or the original expression if the
+  /// conversion is not needed.
+  /// @param inst the SPIR-V instruction
+  /// @param first_operand_type the type of the first operand to the instruction
+  /// @param second_operand_expr the second operand of the instruction
+  /// @returns second_operand_expr, or a cast of it
+  TypedExpression RectifySecondOperandSignedness(
+      const spvtools::opt::Instruction& inst,
+      type::Type* first_operand_type,
+      TypedExpression&& second_operand_expr);
+
   /// Returns the "forced" result type for the given SPIR-V instruction.
   /// If the WGSL result type for an operation has a more strict rule than
   /// requried by SPIR-V, then we say the result type is "forced".  This occurs