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>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 0998926..fe3d320 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -4960,28 +4960,27 @@
TypedExpression FunctionEmitter::MakeSimpleSelect(
const spvtools::opt::Instruction& inst) {
auto condition = MakeOperand(inst, 0);
- auto operand1 = MakeOperand(inst, 1);
- auto operand2 = MakeOperand(inst, 2);
+ auto true_value = MakeOperand(inst, 1);
+ auto false_value = MakeOperand(inst, 2);
// SPIR-V validation requires:
// - 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
// 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() ||
op_ty->IsIntegerScalar() || op_ty->Is<Bool>()) {
ast::ExpressionList params;
- params.push_back(operand1.expr);
- params.push_back(operand2.expr);
+ params.push_back(false_value.expr);
+ params.push_back(true_value.expr);
// The condition goes last.
params.push_back(condition.expr);
- return {operand1.type,
- create<ast::CallExpression>(
- Source{},
- create<ast::IdentifierExpression>(
- Source{}, builder_.Symbols().Register("select")),
- std::move(params))};
+ return {op_ty, create<ast::CallExpression>(
+ Source{},
+ create<ast::IdentifierExpression>(
+ Source{}, builder_.Symbols().Register("select")),
+ std::move(params))};
}
return {};
}
diff --git a/src/reader/spirv/function_logical_test.cc b/src/reader/spirv/function_logical_test.cc
index 180e115..2d21a1b 100644
--- a/src/reader/spirv/function_logical_test.cc
+++ b/src/reader/spirv/function_logical_test.cc
@@ -1160,9 +1160,9 @@
Call[not set]{
Identifier[not set]{select}
(
- ScalarConstructor[not set]{true}
ScalarConstructor[not set]{false}
ScalarConstructor[not set]{true}
+ ScalarConstructor[not set]{true}
)
}
}
@@ -1193,8 +1193,8 @@
Call[not set]{
Identifier[not set]{select}
(
- ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u}
+ ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{true}
)
}
@@ -1226,8 +1226,8 @@
Call[not set]{
Identifier[not set]{select}
(
- ScalarConstructor[not set]{50.000000}
ScalarConstructor[not set]{60.000000}
+ ScalarConstructor[not set]{50.000000}
ScalarConstructor[not set]{true}
)
}
@@ -1264,13 +1264,13 @@
(
TypeConstructor[not set]{
__vec_2__u32
- ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u}
+ ScalarConstructor[not set]{10u}
}
TypeConstructor[not set]{
__vec_2__u32
- ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u}
+ ScalarConstructor[not set]{20u}
}
ScalarConstructor[not set]{true}
)
@@ -1311,13 +1311,13 @@
(
TypeConstructor[not set]{
__vec_2__u32
- ScalarConstructor[not set]{10u}
ScalarConstructor[not set]{20u}
+ ScalarConstructor[not set]{10u}
}
TypeConstructor[not set]{
__vec_2__u32
- ScalarConstructor[not set]{20u}
ScalarConstructor[not set]{10u}
+ ScalarConstructor[not set]{20u}
}
TypeConstructor[not set]{
__vec_2__bool
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index dbaed5a..0a04858 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -1247,8 +1247,8 @@
bool GeneratorImpl::EmitSelectCall(std::ostream& out,
ast::CallExpression* expr) {
- auto* expr_true = expr->params()[0];
- auto* expr_false = expr->params()[1];
+ auto* expr_false = expr->params()[0];
+ auto* expr_true = expr->params()[1];
auto* expr_cond = expr->params()[2];
ScopedParen paren(out);
if (!EmitExpression(out, expr_cond)) {
diff --git a/src/writer/hlsl/generator_impl_intrinsic_test.cc b/src/writer/hlsl/generator_impl_intrinsic_test.cc
index 1d5407a..7bc6289 100644
--- a/src/writer/hlsl/generator_impl_intrinsic_test.cc
+++ b/src/writer/hlsl/generator_impl_intrinsic_test.cc
@@ -281,7 +281,7 @@
gen.increment_indent();
std::stringstream out;
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) {
@@ -293,7 +293,7 @@
gen.increment_indent();
std::stringstream out;
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) {
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index 9382362..b647fa3 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -2448,8 +2448,8 @@
case IntrinsicType::kSelect: {
// Note: Argument order is different in WGSL and SPIR-V
auto cond_id = get_param_as_value_id(2);
- auto true_id = get_param_as_value_id(0);
- auto false_id = get_param_as_value_id(1);
+ auto true_id = get_param_as_value_id(1);
+ auto false_id = get_param_as_value_id(0);
if (!cond_id || !true_id || !false_id) {
return 0;
}
