reader: fix ordering of select operands

Updates:
spirv-reader
hlsl-writer
spirv-writer

Bug: tint:1001
Change-Id: I7359d35c3384e164d149a6af03cbdc959a2eea76
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/58400
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: David Neto <dneto@google.com>
This commit is contained in:
David Neto 2021-07-16 18:17:05 +00:00 committed by Tint LUCI CQ
parent 29c70796d3
commit 5f994247a2
5 changed files with 24 additions and 25 deletions

View File

@ -4960,24 +4960,23 @@ TypedExpression FunctionEmitter::MakeIntrinsicCall(
TypedExpression FunctionEmitter::MakeSimpleSelect( TypedExpression FunctionEmitter::MakeSimpleSelect(
const spvtools::opt::Instruction& inst) { const spvtools::opt::Instruction& inst) {
auto condition = MakeOperand(inst, 0); auto condition = MakeOperand(inst, 0);
auto operand1 = MakeOperand(inst, 1); auto true_value = MakeOperand(inst, 1);
auto operand2 = MakeOperand(inst, 2); auto false_value = MakeOperand(inst, 2);
// SPIR-V validation requires: // SPIR-V validation requires:
// - the condition to be bool or bool vector, so we don't check it here. // - the condition to be bool or bool vector, so we don't check it here.
// - operand1, operand2, and result type to match. // - true_value false_value, and result type to match.
// - you can't select over pointers or pointer vectors, unless you also have // - you can't select over pointers or pointer vectors, unless you also have
// a VariablePointers* capability, which is not allowed in by WebGPU. // a VariablePointers* capability, which is not allowed in by WebGPU.
auto* op_ty = operand1.type; auto* op_ty = true_value.type;
if (op_ty->Is<Vector>() || op_ty->IsFloatScalar() || if (op_ty->Is<Vector>() || op_ty->IsFloatScalar() ||
op_ty->IsIntegerScalar() || op_ty->Is<Bool>()) { op_ty->IsIntegerScalar() || op_ty->Is<Bool>()) {
ast::ExpressionList params; ast::ExpressionList params;
params.push_back(operand1.expr); params.push_back(false_value.expr);
params.push_back(operand2.expr); params.push_back(true_value.expr);
// The condition goes last. // The condition goes last.
params.push_back(condition.expr); params.push_back(condition.expr);
return {operand1.type, return {op_ty, create<ast::CallExpression>(
create<ast::CallExpression>(
Source{}, Source{},
create<ast::IdentifierExpression>( create<ast::IdentifierExpression>(
Source{}, builder_.Symbols().Register("select")), Source{}, builder_.Symbols().Register("select")),

View File

@ -1160,9 +1160,9 @@ TEST_F(SpvLogicalTest, Select_BoolCond_BoolParams) {
Call[not set]{ Call[not set]{
Identifier[not set]{select} Identifier[not set]{select}
( (
ScalarConstructor[not set]{true}
ScalarConstructor[not set]{false} ScalarConstructor[not set]{false}
ScalarConstructor[not set]{true} ScalarConstructor[not set]{true}
ScalarConstructor[not set]{true}
) )
} }
} }
@ -1193,8 +1193,8 @@ TEST_F(SpvLogicalTest, Select_BoolCond_IntScalarParams) {
Call[not set]{ Call[not set]{
Identifier[not set]{select} Identifier[not set]{select}
( (
ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u} ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{true} ScalarConstructor[not set]{true}
) )
} }
@ -1226,8 +1226,8 @@ TEST_F(SpvLogicalTest, Select_BoolCond_FloatScalarParams) {
Call[not set]{ Call[not set]{
Identifier[not set]{select} Identifier[not set]{select}
( (
ScalarConstructor[not set]{50.000000}
ScalarConstructor[not set]{60.000000} ScalarConstructor[not set]{60.000000}
ScalarConstructor[not set]{50.000000}
ScalarConstructor[not set]{true} ScalarConstructor[not set]{true}
) )
} }
@ -1264,13 +1264,13 @@ TEST_F(SpvLogicalTest, Select_BoolCond_VectorParams) {
( (
TypeConstructor[not set]{ TypeConstructor[not set]{
__vec_2__u32 __vec_2__u32
ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u} ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u}
} }
TypeConstructor[not set]{ TypeConstructor[not set]{
__vec_2__u32 __vec_2__u32
ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u} ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u}
} }
ScalarConstructor[not set]{true} ScalarConstructor[not set]{true}
) )
@ -1311,13 +1311,13 @@ TEST_F(SpvLogicalTest, Select_VecBoolCond_VectorParams) {
( (
TypeConstructor[not set]{ TypeConstructor[not set]{
__vec_2__u32 __vec_2__u32
ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u} ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u}
} }
TypeConstructor[not set]{ TypeConstructor[not set]{
__vec_2__u32 __vec_2__u32
ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u} ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u}
} }
TypeConstructor[not set]{ TypeConstructor[not set]{
__vec_2__bool __vec_2__bool

View File

@ -1247,8 +1247,8 @@ bool GeneratorImpl::EmitWorkgroupAtomicCall(std::ostream& out,
bool GeneratorImpl::EmitSelectCall(std::ostream& out, bool GeneratorImpl::EmitSelectCall(std::ostream& out,
ast::CallExpression* expr) { ast::CallExpression* expr) {
auto* expr_true = expr->params()[0]; auto* expr_false = expr->params()[0];
auto* expr_false = expr->params()[1]; auto* expr_true = expr->params()[1];
auto* expr_cond = expr->params()[2]; auto* expr_cond = expr->params()[2];
ScopedParen paren(out); ScopedParen paren(out);
if (!EmitExpression(out, expr_cond)) { if (!EmitExpression(out, expr_cond)) {

View File

@ -281,7 +281,7 @@ TEST_F(HlslGeneratorImplTest_Intrinsic, Select_Scalar) {
gen.increment_indent(); gen.increment_indent();
std::stringstream out; std::stringstream out;
ASSERT_TRUE(gen.EmitExpression(out, call)) << gen.error(); ASSERT_TRUE(gen.EmitExpression(out, call)) << gen.error();
EXPECT_EQ(out.str(), "(true ? 1.0f : 2.0f)"); EXPECT_EQ(out.str(), "(true ? 2.0f : 1.0f)");
} }
TEST_F(HlslGeneratorImplTest_Intrinsic, Select_Vector) { TEST_F(HlslGeneratorImplTest_Intrinsic, Select_Vector) {
@ -293,7 +293,7 @@ TEST_F(HlslGeneratorImplTest_Intrinsic, Select_Vector) {
gen.increment_indent(); gen.increment_indent();
std::stringstream out; std::stringstream out;
ASSERT_TRUE(gen.EmitExpression(out, call)) << gen.error(); ASSERT_TRUE(gen.EmitExpression(out, call)) << gen.error();
EXPECT_EQ(out.str(), "(bool2(true, false) ? int2(1, 2) : int2(3, 4))"); EXPECT_EQ(out.str(), "(bool2(true, false) ? int2(3, 4) : int2(1, 2))");
} }
TEST_F(HlslGeneratorImplTest_Intrinsic, Modf_Scalar) { TEST_F(HlslGeneratorImplTest_Intrinsic, Modf_Scalar) {

View File

@ -2448,8 +2448,8 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call,
case IntrinsicType::kSelect: { case IntrinsicType::kSelect: {
// Note: Argument order is different in WGSL and SPIR-V // Note: Argument order is different in WGSL and SPIR-V
auto cond_id = get_param_as_value_id(2); auto cond_id = get_param_as_value_id(2);
auto true_id = get_param_as_value_id(0); auto true_id = get_param_as_value_id(1);
auto false_id = get_param_as_value_id(1); auto false_id = get_param_as_value_id(0);
if (!cond_id || !true_id || !false_id) { if (!cond_id || !true_id || !false_id) {
return 0; return 0;
} }