Remove outerProduct.
The community decided to remove outerProduct from WGSL. This Cl removes
the pieces from Tint.
Change-Id: Ib1735867e4a7ca852a72549fc8c9bd86e8de22b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37600
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/docs/translations.md b/docs/translations.md
index d6ff55e..5688952 100644
--- a/docs/translations.md
+++ b/docs/translations.md
@@ -88,7 +88,6 @@
| mix | GLSLstd450FMix | mix | mix |
| modf | GLSLstd450Modf | | |
| normalize | GLSLstd450Normalize | normalize | normalize |
-| outerProduct | SpvOpOuterProduct | | |
| pow | GLSLstd450Pow | pow | pow |
| reflect | GLSLstd450Reflect | reflect | reflect |
| reverseBits | SpvOpBitReverse | reverse_bits | reversebits |
diff --git a/src/ast/intrinsic.cc b/src/ast/intrinsic.cc
index c4ce9c1..73f3ab3 100644
--- a/src/ast/intrinsic.cc
+++ b/src/ast/intrinsic.cc
@@ -162,9 +162,6 @@
case Intrinsic::kNormalize:
out << "normalize";
break;
- case Intrinsic::kOuterProduct:
- out << "outerProduct";
- break;
case Intrinsic::kPow:
out << "pow";
break;
diff --git a/src/ast/intrinsic.h b/src/ast/intrinsic.h
index 568c9ee..9de55b1 100644
--- a/src/ast/intrinsic.h
+++ b/src/ast/intrinsic.h
@@ -70,7 +70,6 @@
kMix,
kModf,
kNormalize,
- kOuterProduct,
kPow,
kReflect,
kReverseBits,
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index fcfcc33..93e7cd8 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -472,8 +472,6 @@
return ast::Intrinsic::kReverseBits;
case SpvOpDot:
return ast::Intrinsic::kDot;
- case SpvOpOuterProduct:
- return ast::Intrinsic::kOuterProduct;
default:
break;
}
diff --git a/src/reader/spirv/function_arithmetic_test.cc b/src/reader/spirv/function_arithmetic_test.cc
index b474a27..044fca5 100644
--- a/src/reader/spirv/function_arithmetic_test.cc
+++ b/src/reader/spirv/function_arithmetic_test.cc
@@ -1099,38 +1099,6 @@
<< ToString(p->get_module(), fe.ast_body());
}
-TEST_F(SpvBinaryArithTestBasic, OuterProduct) {
- const auto assembly = CommonTypes() + R"(
- %100 = OpFunction %void None %voidfn
- %entry = OpLabel
- %1 = OpCopyObject %v2float %v2float_50_60
- %2 = OpCopyObject %v2float %v2float_60_50
- %3 = OpOuterProduct %m2v2float %1 %2
- OpReturn
- OpFunctionEnd
-)";
- auto p = parser(test::Assemble(assembly));
- ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << assembly;
- FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
- EXPECT_TRUE(fe.EmitBody()) << p->error();
- EXPECT_THAT(ToString(p->get_module(), fe.ast_body()),
- HasSubstr(R"(VariableConst{
- x_3
- none
- __mat_2_2__f32
- {
- Call[not set]{
- Identifier[not set]{outerProduct}
- (
- Identifier[not set]{x_1}
- Identifier[not set]{x_2}
- )
- }
- }
- })"))
- << ToString(p->get_module(), fe.ast_body());
-}
-
// TODO(dneto): OpSRem. Missing from WGSL
// https://github.com/gpuweb/gpuweb/issues/702
diff --git a/src/type_determiner.cc b/src/type_determiner.cc
index eaa95bf..b8aab37 100644
--- a/src/type_determiner.cc
+++ b/src/type_determiner.cc
@@ -721,28 +721,6 @@
expr->func()->set_result_type(mod_->create<ast::type::F32>());
return true;
}
- if (ident->intrinsic() == ast::Intrinsic::kOuterProduct) {
- if (expr->params().size() != 2) {
- set_error(expr->source(), "incorrect number of parameters for " +
- mod_->SymbolToName(ident->symbol()));
- return false;
- }
-
- auto* param0_type = expr->params()[0]->result_type()->UnwrapPtrIfNeeded();
- auto* param1_type = expr->params()[1]->result_type()->UnwrapPtrIfNeeded();
- if (!param0_type->Is<ast::type::Vector>() ||
- !param1_type->Is<ast::type::Vector>()) {
- set_error(expr->source(), "invalid parameter type for " +
- mod_->SymbolToName(ident->symbol()));
- return false;
- }
-
- expr->func()->set_result_type(mod_->create<ast::type::Matrix>(
- mod_->create<ast::type::F32>(),
- param0_type->As<ast::type::Vector>()->size(),
- param1_type->As<ast::type::Vector>()->size()));
- return true;
- }
if (ident->intrinsic() == ast::Intrinsic::kSelect) {
if (expr->params().size() != 3) {
set_error(expr->source(), "incorrect number of parameters for " +
@@ -1017,8 +995,6 @@
ident->set_intrinsic(ast::Intrinsic::kModf);
} else if (name == "normalize") {
ident->set_intrinsic(ast::Intrinsic::kNormalize);
- } else if (name == "outerProduct") {
- ident->set_intrinsic(ast::Intrinsic::kOuterProduct);
} else if (name == "pow") {
ident->set_intrinsic(ast::Intrinsic::kPow);
} else if (name == "reflect") {
diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc
index 46545c4..87f6ed0 100644
--- a/src/type_determiner_test.cc
+++ b/src/type_determiner_test.cc
@@ -1617,55 +1617,6 @@
"incorrect number of parameters for select expected 3 got 4");
}
-TEST_F(TypeDeterminerTest, Intrinsic_OuterProduct) {
- auto* var1 = Var( // source
- "v3", ast::StorageClass::kNone, ty.vec3<f32>());
- auto* var2 = Var( // source
- "v2", ast::StorageClass::kNone, ty.vec2<f32>());
- mod->AddGlobalVariable(var1);
- mod->AddGlobalVariable(var2);
-
- auto* expr = Call("outerProduct", "v3", "v2");
-
- // Register the variable
- EXPECT_TRUE(td()->Determine());
- EXPECT_TRUE(td()->DetermineResultType(expr));
-
- ASSERT_NE(expr->result_type(), nullptr);
- ASSERT_TRUE(expr->result_type()->Is<ast::type::Matrix>());
-
- auto* mat = expr->result_type()->As<ast::type::Matrix>();
- EXPECT_TRUE(mat->type()->Is<ast::type::F32>());
- EXPECT_EQ(mat->rows(), 3u);
- EXPECT_EQ(mat->columns(), 2u);
-}
-
-TEST_F(TypeDeterminerTest, Intrinsic_OuterProduct_TooFewParams) {
- auto* var2 = Var( // source
- "v2", ast::StorageClass::kNone, ty.vec2<f32>());
- mod->AddGlobalVariable(var2);
-
- auto* expr = Call("outerProduct", "v2");
-
- // Register the variable
- EXPECT_TRUE(td()->Determine());
- EXPECT_FALSE(td()->DetermineResultType(expr));
- EXPECT_EQ(td()->error(), "incorrect number of parameters for outerProduct");
-}
-
-TEST_F(TypeDeterminerTest, Intrinsic_OuterProduct_TooManyParams) {
- auto* var2 = Var( // source
- "v2", ast::StorageClass::kNone, ty.vec2<f32>());
- mod->AddGlobalVariable(var2);
-
- auto* expr = Call("outerProduct", "v2", "v2", "v2");
-
- // Register the variable
- EXPECT_TRUE(td()->Determine());
- EXPECT_FALSE(td()->DetermineResultType(expr));
- EXPECT_EQ(td()->error(), "incorrect number of parameters for outerProduct");
-}
-
using UnaryOpExpressionTest = TypeDeterminerTestWithParam<ast::UnaryOp>;
TEST_P(UnaryOpExpressionTest, Expr_UnaryOp) {
auto op = GetParam();
@@ -1798,7 +1749,6 @@
IntrinsicData{"mix", ast::Intrinsic::kMix},
IntrinsicData{"modf", ast::Intrinsic::kModf},
IntrinsicData{"normalize", ast::Intrinsic::kNormalize},
- IntrinsicData{"outerProduct", ast::Intrinsic::kOuterProduct},
IntrinsicData{"pow", ast::Intrinsic::kPow},
IntrinsicData{"reflect", ast::Intrinsic::kReflect},
IntrinsicData{"reverseBits", ast::Intrinsic::kReverseBits},
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index d95769f..4c6067b 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -595,59 +595,6 @@
} else if (ident->intrinsic() == ast::Intrinsic::kIsNormal) {
error_ = "is_normal not supported in HLSL backend yet";
return false;
- } else if (ident->intrinsic() == ast::Intrinsic::kOuterProduct) {
- error_ = "outer_product not supported yet";
- return false;
- // TODO(dsinclair): This gets tricky. We need to generate two variables to
- // hold the outer_product expressions, but we maybe inside an expression
- // ourselves. So, this will need to, possibly, output the variables
- // _before_ the expression which contains the outer product.
- //
- // This then has the follow on, what if we have `(false &&
- // outer_product())` in that case, we shouldn't evaluate the expressions
- // at all because of short circuting.
- //
- // So .... this turns out to be hard ...
-
- // // We create variables to hold the two parameters in case they're
- // // function calls with side effects.
- // auto* param0 = param[0].get();
- // auto* name0 = generate_name("outer_product_expr_0");
-
- // auto* param1 = param[1].get();
- // auto* name1 = generate_name("outer_product_expr_1");
-
- // make_indent(out);
- // if (!EmitType(out, expr->result_type(), "")) {
- // return false;
- // }
- // out << "(";
-
- // auto param1_type = params[1]->result_type()->UnwrapPtrIfNeeded();
- // if (!param1_type->Is<ast::type::Vector>()) {
- // error_ = "invalid param type in outer_product got: " +
- // param1_type->type_name();
- // return false;
- // }
-
- // for (uint32_t i = 0; i <
- // param1_type->As<ast::type::Vector>()->size(); ++i) {
- // if (i > 0) {
- // out << ", ";
- // }
-
- // if (!EmitExpression(pre, out, params[0].get())) {
- // return false;
- // }
- // out << " * ";
-
- // if (!EmitExpression(pre, out, params[1].get())) {
- // return false;
- // }
- // out << "[" << i << "]";
- // }
-
- // out << ")";
} else {
auto name = generate_intrinsic_name(ident->intrinsic());
if (name.empty()) {
diff --git a/src/writer/hlsl/generator_impl_intrinsic_test.cc b/src/writer/hlsl/generator_impl_intrinsic_test.cc
index 1841ef6..2658f74 100644
--- a/src/writer/hlsl/generator_impl_intrinsic_test.cc
+++ b/src/writer/hlsl/generator_impl_intrinsic_test.cc
@@ -70,26 +70,6 @@
FAIL();
}
-TEST_F(HlslGeneratorImplTest_Intrinsic, DISABLED_Intrinsic_OuterProduct) {
- auto* a = Var("a", ast::StorageClass::kNone, ty.vec2<f32>());
- auto* b = Var("b", ast::StorageClass::kNone, ty.vec3<f32>());
-
- auto* call = Call("outer_product", "a", "b");
-
- td.RegisterVariableForTesting(a);
- td.RegisterVariableForTesting(b);
-
- mod->AddGlobalVariable(a);
- mod->AddGlobalVariable(b);
-
- ASSERT_TRUE(td.Determine()) << td.error();
- ASSERT_TRUE(td.DetermineResultType(call)) << td.error();
-
- gen.increment_indent();
- ASSERT_TRUE(gen.EmitExpression(pre, out, call)) << gen.error();
- EXPECT_EQ(result(), " float3x2(a * b[0], a * b[1], a * b[2])");
-}
-
TEST_F(HlslGeneratorImplTest_Intrinsic, Intrinsic_Bad_Name) {
EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), "");
}
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 5343a11..a9509a8 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -487,89 +487,34 @@
}
if (ident->IsIntrinsic()) {
- const auto& params = expr->params();
- if (ident->intrinsic() == ast::Intrinsic::kOuterProduct) {
- error_ = "outer_product not supported yet";
- return false;
- // TODO(dsinclair): This gets tricky. We need to generate two variables to
- // hold the outer_product expressions, but we maybe inside an expression
- // ourselves. So, this will need to, possibly, output the variables
- // _before_ the expression which contains the outer product.
- //
- // This then has the follow on, what if we have `(false &&
- // outer_product())` in that case, we shouldn't evaluate the expressions
- // at all because of short circuting.
- //
- // So .... this turns out to be hard ...
-
- // // We create variables to hold the two parameters in case they're
- // // function calls with side effects.
- // auto* param0 = param[0].get();
- // auto* name0 = generate_name("outer_product_expr_0");
-
- // auto* param1 = param[1].get();
- // auto* name1 = generate_name("outer_product_expr_1");
-
- // make_indent();
- // if (!EmitType(expr->result_type(), "")) {
- // return false;
- // }
- // out_ << "(";
-
- // auto param1_type = params[1]->result_type()->UnwrapPtrIfNeeded();
- // if (!param1_type->Is<ast::type::Vector>()) {
- // error_ = "invalid param type in outer_product got: " +
- // param1_type->type_name();
- // return false;
- // }
-
- // for (uint32_t i = 0; i <
- // param1_type->As<ast::type::Vector>()->size(); ++i) {
- // if (i > 0) {
- // out_ << ", ";
- // }
-
- // if (!EmitExpression(params[0].get())) {
- // return false;
- // }
- // out_ << " * ";
-
- // if (!EmitExpression(params[1].get())) {
- // return false;
- // }
- // out_ << "[" << i << "]";
- // }
-
- // out_ << ")";
- } else {
- auto name = generate_intrinsic_name(ident->intrinsic());
+ auto name = generate_intrinsic_name(ident->intrinsic());
+ if (name.empty()) {
+ if (ast::intrinsic::IsTextureIntrinsic(ident->intrinsic())) {
+ return EmitTextureCall(expr);
+ }
+ name = generate_builtin_name(ident);
if (name.empty()) {
- if (ast::intrinsic::IsTextureIntrinsic(ident->intrinsic())) {
- return EmitTextureCall(expr);
- }
- name = generate_builtin_name(ident);
- if (name.empty()) {
- return false;
- }
+ return false;
}
-
- make_indent();
- out_ << name << "(";
-
- bool first = true;
- for (auto* param : params) {
- if (!first) {
- out_ << ", ";
- }
- first = false;
-
- if (!EmitExpression(param)) {
- return false;
- }
- }
-
- out_ << ")";
}
+
+ make_indent();
+ out_ << name << "(";
+
+ bool first = true;
+ const auto& params = expr->params();
+ for (auto* param : params) {
+ if (!first) {
+ out_ << ", ";
+ }
+ first = false;
+
+ if (!EmitExpression(param)) {
+ return false;
+ }
+ }
+
+ out_ << ")";
return true;
}
diff --git a/src/writer/msl/generator_impl_intrinsic_test.cc b/src/writer/msl/generator_impl_intrinsic_test.cc
index af661cb..b1f0e92 100644
--- a/src/writer/msl/generator_impl_intrinsic_test.cc
+++ b/src/writer/msl/generator_impl_intrinsic_test.cc
@@ -65,25 +65,6 @@
IntrinsicData{ast::Intrinsic::kReverseBits, "reverse_bits"},
IntrinsicData{ast::Intrinsic::kSelect, "select"}));
-TEST_F(MslGeneratorImplTest, DISABLED_Intrinsic_OuterProduct) {
- auto* a = Var("a", ast::StorageClass::kNone, ty.vec2<f32>());
- auto* b = Var("b", ast::StorageClass::kNone, ty.vec3<f32>());
-
- auto* call = Call("outer_product", "a", "b");
- td.RegisterVariableForTesting(a);
- td.RegisterVariableForTesting(b);
-
- mod->AddGlobalVariable(a);
- mod->AddGlobalVariable(b);
-
- ASSERT_TRUE(td.Determine()) << td.error();
- ASSERT_TRUE(td.DetermineResultType(call)) << td.error();
-
- gen.increment_indent();
- ASSERT_TRUE(gen.EmitExpression(call)) << gen.error();
- EXPECT_EQ(gen.result(), " float3x2(a * b[0], a * b[1], a * b[2])");
-}
-
TEST_F(MslGeneratorImplTest, Intrinsic_Bad_Name) {
EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), "");
}
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index 1bdd166..598a0db 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -1940,8 +1940,6 @@
op = spv::Op::OpIsInf;
} else if (intrinsic == ast::Intrinsic::kIsNan) {
op = spv::Op::OpIsNan;
- } else if (intrinsic == ast::Intrinsic::kOuterProduct) {
- op = spv::Op::OpOuterProduct;
} else if (intrinsic == ast::Intrinsic::kReverseBits) {
op = spv::Op::OpBitReverse;
} else if (intrinsic == ast::Intrinsic::kSelect) {
diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc
index c2dde98..654317b 100644
--- a/src/writer/spirv/builder_intrinsic_test.cc
+++ b/src/writer/spirv/builder_intrinsic_test.cc
@@ -349,38 +349,6 @@
IntrinsicData{"fwidthFine", "OpFwidthFine"},
IntrinsicData{"fwidthCoarse", "OpFwidthCoarse"}));
-TEST_F(IntrinsicBuilderTest, Call_OuterProduct) {
- auto* v2 = Var("v2", ast::StorageClass::kPrivate, ty.vec2<f32>());
- auto* v3 = Var("v3", ast::StorageClass::kPrivate, ty.vec3<f32>());
-
- auto* expr = Call("outerProduct", "v2", "v3");
-
- ASSERT_TRUE(td.DetermineResultType(expr)) << td.error();
-
- b.push_function(Function{});
- ASSERT_TRUE(b.GenerateGlobalVariable(v2)) << b.error();
- ASSERT_TRUE(b.GenerateGlobalVariable(v3)) << b.error();
-
- EXPECT_EQ(b.GenerateCallExpression(expr), 10u) << b.error();
-
- EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeFloat 32
-%3 = OpTypeVector %4 2
-%2 = OpTypePointer Private %3
-%5 = OpConstantNull %3
-%1 = OpVariable %2 Private %5
-%8 = OpTypeVector %4 3
-%7 = OpTypePointer Private %8
-%9 = OpConstantNull %8
-%6 = OpVariable %7 Private %9
-%11 = OpTypeMatrix %3 3
-)");
- EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
- R"(%12 = OpLoad %3 %1
-%13 = OpLoad %8 %6
-%10 = OpOuterProduct %11 %12 %13
-)");
-}
-
TEST_F(IntrinsicBuilderTest, Call_Select) {
auto* v3 = Var("v3", ast::StorageClass::kPrivate, ty.vec3<f32>());
auto* bool_v3 = Var("bool_v3", ast::StorageClass::kPrivate, ty.vec3<bool>());
