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>
This commit is contained in:
dan sinclair 2021-01-12 22:00:58 +00:00 committed by Commit Bot service account
parent 8f4784006e
commit 1c200cffd0
13 changed files with 25 additions and 319 deletions

View File

@ -88,7 +88,6 @@ decorated with `NonWritable` or each member of the struct can be decorated with
| mix | GLSLstd450FMix | mix | mix | | mix | GLSLstd450FMix | mix | mix |
| modf | GLSLstd450Modf | | | | modf | GLSLstd450Modf | | |
| normalize | GLSLstd450Normalize | normalize | normalize | | normalize | GLSLstd450Normalize | normalize | normalize |
| outerProduct | SpvOpOuterProduct | | |
| pow | GLSLstd450Pow | pow | pow | | pow | GLSLstd450Pow | pow | pow |
| reflect | GLSLstd450Reflect | reflect | reflect | | reflect | GLSLstd450Reflect | reflect | reflect |
| reverseBits | SpvOpBitReverse | reverse_bits | reversebits | | reverseBits | SpvOpBitReverse | reverse_bits | reversebits |

View File

@ -162,9 +162,6 @@ std::ostream& operator<<(std::ostream& out, Intrinsic i) {
case Intrinsic::kNormalize: case Intrinsic::kNormalize:
out << "normalize"; out << "normalize";
break; break;
case Intrinsic::kOuterProduct:
out << "outerProduct";
break;
case Intrinsic::kPow: case Intrinsic::kPow:
out << "pow"; out << "pow";
break; break;

View File

@ -70,7 +70,6 @@ enum class Intrinsic {
kMix, kMix,
kModf, kModf,
kNormalize, kNormalize,
kOuterProduct,
kPow, kPow,
kReflect, kReflect,
kReverseBits, kReverseBits,

View File

@ -472,8 +472,6 @@ ast::Intrinsic GetIntrinsic(SpvOp opcode) {
return ast::Intrinsic::kReverseBits; return ast::Intrinsic::kReverseBits;
case SpvOpDot: case SpvOpDot:
return ast::Intrinsic::kDot; return ast::Intrinsic::kDot;
case SpvOpOuterProduct:
return ast::Intrinsic::kOuterProduct;
default: default:
break; break;
} }

View File

@ -1099,38 +1099,6 @@ TEST_F(SpvBinaryArithTestBasic, Dot) {
<< ToString(p->get_module(), fe.ast_body()); << 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 // TODO(dneto): OpSRem. Missing from WGSL
// https://github.com/gpuweb/gpuweb/issues/702 // https://github.com/gpuweb/gpuweb/issues/702

View File

@ -721,28 +721,6 @@ bool TypeDeterminer::DetermineIntrinsic(ast::IdentifierExpression* ident,
expr->func()->set_result_type(mod_->create<ast::type::F32>()); expr->func()->set_result_type(mod_->create<ast::type::F32>());
return true; 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 (ident->intrinsic() == ast::Intrinsic::kSelect) {
if (expr->params().size() != 3) { if (expr->params().size() != 3) {
set_error(expr->source(), "incorrect number of parameters for " + set_error(expr->source(), "incorrect number of parameters for " +
@ -1017,8 +995,6 @@ bool TypeDeterminer::SetIntrinsicIfNeeded(ast::IdentifierExpression* ident) {
ident->set_intrinsic(ast::Intrinsic::kModf); ident->set_intrinsic(ast::Intrinsic::kModf);
} else if (name == "normalize") { } else if (name == "normalize") {
ident->set_intrinsic(ast::Intrinsic::kNormalize); ident->set_intrinsic(ast::Intrinsic::kNormalize);
} else if (name == "outerProduct") {
ident->set_intrinsic(ast::Intrinsic::kOuterProduct);
} else if (name == "pow") { } else if (name == "pow") {
ident->set_intrinsic(ast::Intrinsic::kPow); ident->set_intrinsic(ast::Intrinsic::kPow);
} else if (name == "reflect") { } else if (name == "reflect") {

View File

@ -1617,55 +1617,6 @@ TEST_F(TypeDeterminerTest, Intrinsic_Select_TooManyParams) {
"incorrect number of parameters for select expected 3 got 4"); "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>; using UnaryOpExpressionTest = TypeDeterminerTestWithParam<ast::UnaryOp>;
TEST_P(UnaryOpExpressionTest, Expr_UnaryOp) { TEST_P(UnaryOpExpressionTest, Expr_UnaryOp) {
auto op = GetParam(); auto op = GetParam();
@ -1798,7 +1749,6 @@ INSTANTIATE_TEST_SUITE_P(
IntrinsicData{"mix", ast::Intrinsic::kMix}, IntrinsicData{"mix", ast::Intrinsic::kMix},
IntrinsicData{"modf", ast::Intrinsic::kModf}, IntrinsicData{"modf", ast::Intrinsic::kModf},
IntrinsicData{"normalize", ast::Intrinsic::kNormalize}, IntrinsicData{"normalize", ast::Intrinsic::kNormalize},
IntrinsicData{"outerProduct", ast::Intrinsic::kOuterProduct},
IntrinsicData{"pow", ast::Intrinsic::kPow}, IntrinsicData{"pow", ast::Intrinsic::kPow},
IntrinsicData{"reflect", ast::Intrinsic::kReflect}, IntrinsicData{"reflect", ast::Intrinsic::kReflect},
IntrinsicData{"reverseBits", ast::Intrinsic::kReverseBits}, IntrinsicData{"reverseBits", ast::Intrinsic::kReverseBits},

View File

@ -595,59 +595,6 @@ bool GeneratorImpl::EmitCall(std::ostream& pre,
} else if (ident->intrinsic() == ast::Intrinsic::kIsNormal) { } else if (ident->intrinsic() == ast::Intrinsic::kIsNormal) {
error_ = "is_normal not supported in HLSL backend yet"; error_ = "is_normal not supported in HLSL backend yet";
return false; 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 { } else {
auto name = generate_intrinsic_name(ident->intrinsic()); auto name = generate_intrinsic_name(ident->intrinsic());
if (name.empty()) { if (name.empty()) {

View File

@ -70,26 +70,6 @@ TEST_F(HlslGeneratorImplTest_Intrinsic, DISABLED_Intrinsic_Select) {
FAIL(); 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) { TEST_F(HlslGeneratorImplTest_Intrinsic, Intrinsic_Bad_Name) {
EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), ""); EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), "");
} }

View File

@ -487,61 +487,6 @@ bool GeneratorImpl::EmitCall(ast::CallExpression* expr) {
} }
if (ident->IsIntrinsic()) { 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 (name.empty()) {
if (ast::intrinsic::IsTextureIntrinsic(ident->intrinsic())) { if (ast::intrinsic::IsTextureIntrinsic(ident->intrinsic())) {
@ -557,6 +502,7 @@ bool GeneratorImpl::EmitCall(ast::CallExpression* expr) {
out_ << name << "("; out_ << name << "(";
bool first = true; bool first = true;
const auto& params = expr->params();
for (auto* param : params) { for (auto* param : params) {
if (!first) { if (!first) {
out_ << ", "; out_ << ", ";
@ -569,7 +515,6 @@ bool GeneratorImpl::EmitCall(ast::CallExpression* expr) {
} }
out_ << ")"; out_ << ")";
}
return true; return true;
} }

View File

@ -65,25 +65,6 @@ INSTANTIATE_TEST_SUITE_P(
IntrinsicData{ast::Intrinsic::kReverseBits, "reverse_bits"}, IntrinsicData{ast::Intrinsic::kReverseBits, "reverse_bits"},
IntrinsicData{ast::Intrinsic::kSelect, "select"})); 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) { TEST_F(MslGeneratorImplTest, Intrinsic_Bad_Name) {
EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), ""); EXPECT_EQ(gen.generate_intrinsic_name(ast::Intrinsic::kNone), "");
} }

View File

@ -1940,8 +1940,6 @@ uint32_t Builder::GenerateIntrinsic(ast::IdentifierExpression* ident,
op = spv::Op::OpIsInf; op = spv::Op::OpIsInf;
} else if (intrinsic == ast::Intrinsic::kIsNan) { } else if (intrinsic == ast::Intrinsic::kIsNan) {
op = spv::Op::OpIsNan; op = spv::Op::OpIsNan;
} else if (intrinsic == ast::Intrinsic::kOuterProduct) {
op = spv::Op::OpOuterProduct;
} else if (intrinsic == ast::Intrinsic::kReverseBits) { } else if (intrinsic == ast::Intrinsic::kReverseBits) {
op = spv::Op::OpBitReverse; op = spv::Op::OpBitReverse;
} else if (intrinsic == ast::Intrinsic::kSelect) { } else if (intrinsic == ast::Intrinsic::kSelect) {

View File

@ -349,38 +349,6 @@ INSTANTIATE_TEST_SUITE_P(
IntrinsicData{"fwidthFine", "OpFwidthFine"}, IntrinsicData{"fwidthFine", "OpFwidthFine"},
IntrinsicData{"fwidthCoarse", "OpFwidthCoarse"})); 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) { TEST_F(IntrinsicBuilderTest, Call_Select) {
auto* v3 = Var("v3", ast::StorageClass::kPrivate, ty.vec3<f32>()); auto* v3 = Var("v3", ast::StorageClass::kPrivate, ty.vec3<f32>());
auto* bool_v3 = Var("bool_v3", ast::StorageClass::kPrivate, ty.vec3<bool>()); auto* bool_v3 = Var("bool_v3", ast::StorageClass::kPrivate, ty.vec3<bool>());