spirv-reader: unwrap references for conversions
Bug: tint:804
Change-Id: I39a4d20de17a5c2ae23e9425d85559b7bb70fd22
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52845
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 61dae65..97d1f18 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -4475,6 +4475,7 @@
if (!arg_expr) {
return {};
}
+ arg_expr.type = arg_expr.type->UnwrapRef();
const Type* expr_type = nullptr;
if ((opcode == SpvOpConvertSToF) || (opcode == SpvOpConvertUToF)) {
@@ -4482,21 +4483,24 @@
expr_type = requested_type;
} else {
Fail() << "operand for conversion to floating point must be integral "
- "scalar or vector";
+ "scalar or vector: "
+ << inst.PrettyPrint();
}
} else if (inst.opcode() == SpvOpConvertFToU) {
if (arg_expr.type->IsFloatScalarOrVector()) {
expr_type = parser_impl_.GetUnsignedIntMatchingShape(arg_expr.type);
} else {
Fail() << "operand for conversion to unsigned integer must be floating "
- "point scalar or vector";
+ "point scalar or vector: "
+ << inst.PrettyPrint();
}
} else if (inst.opcode() == SpvOpConvertFToS) {
if (arg_expr.type->IsFloatScalarOrVector()) {
expr_type = parser_impl_.GetSignedIntMatchingShape(arg_expr.type);
} else {
Fail() << "operand for conversion to signed integer must be floating "
- "point scalar or vector";
+ "point scalar or vector: "
+ << inst.PrettyPrint();
}
}
if (expr_type == nullptr) {
@@ -4506,16 +4510,17 @@
ast::ExpressionList params;
params.push_back(arg_expr.expr);
- TypedExpression result{
- expr_type, create<ast::TypeConstructorExpression>(
- Source{}, expr_type->Build(builder_), std::move(params))};
+ TypedExpression result{expr_type,
+ create<ast::TypeConstructorExpression>(
+ GetSourceForInst(inst), expr_type->Build(builder_),
+ std::move(params))};
if (requested_type == expr_type) {
return result;
}
- return {requested_type,
- create<ast::BitcastExpression>(
- Source{}, requested_type->Build(builder_), result.expr)};
+ return {requested_type, create<ast::BitcastExpression>(
+ GetSourceForInst(inst),
+ requested_type->Build(builder_), result.expr)};
}
bool FunctionEmitter::EmitFunctionCall(const spvtools::opt::Instruction& inst) {
diff --git a/src/reader/spirv/function_conversion_test.cc b/src/reader/spirv/function_conversion_test.cc
index 8dc4999..6d77b27 100644
--- a/src/reader/spirv/function_conversion_test.cc
+++ b/src/reader/spirv/function_conversion_test.cc
@@ -343,8 +343,9 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(), Eq("operand for conversion to floating point must be "
- "integral scalar or vector"));
+ EXPECT_THAT(p->error(),
+ HasSubstr("operand for conversion to floating point must be "
+ "integral scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertUToF_Vector_BadArgType) {
@@ -359,9 +360,10 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(),
- Eq("operand for conversion to floating point must be integral "
- "scalar or vector"));
+ EXPECT_THAT(
+ p->error(),
+ HasSubstr("operand for conversion to floating point must be integral "
+ "scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertUToF_Scalar_FromSigned) {
@@ -484,9 +486,10 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(),
- Eq("operand for conversion to signed integer must be floating "
- "point scalar or vector"));
+ EXPECT_THAT(
+ p->error(),
+ HasSubstr("operand for conversion to signed integer must be floating "
+ "point scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertFToS_Vector_BadArgType) {
@@ -501,9 +504,10 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(),
- Eq("operand for conversion to signed integer must be floating "
- "point scalar or vector"));
+ EXPECT_THAT(
+ p->error(),
+ HasSubstr("operand for conversion to signed integer must be floating "
+ "point scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertFToS_Scalar_ToSigned) {
@@ -626,9 +630,10 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(),
- Eq("operand for conversion to unsigned integer must be floating "
- "point scalar or vector"));
+ EXPECT_THAT(
+ p->error(),
+ HasSubstr("operand for conversion to unsigned integer must be floating "
+ "point scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertFToU_Vector_BadArgType) {
@@ -643,9 +648,10 @@
ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
auto fe = p->function_emitter(100);
EXPECT_FALSE(fe.EmitBody());
- EXPECT_THAT(p->error(),
- Eq("operand for conversion to unsigned integer must be floating "
- "point scalar or vector"));
+ EXPECT_THAT(
+ p->error(),
+ HasSubstr("operand for conversion to unsigned integer must be floating "
+ "point scalar or vector"));
}
TEST_F(SpvUnaryConversionTest, ConvertFToU_Scalar_ToSigned_IsError) {
@@ -732,6 +738,56 @@
})"));
}
+TEST_F(SpvUnaryConversionTest, ConvertFToU_HoistedValue) {
+ // From crbug.com/tint/804
+ const auto assembly = Preamble() + R"(
+
+%100 = OpFunction %void None %voidfn
+%10 = OpLabel
+OpBranch %30
+
+%30 = OpLabel
+OpLoopMerge %90 %80 None
+OpBranchConditional %true %90 %40
+
+%40 = OpLabel
+OpSelectionMerge %50 None
+OpBranchConditional %true %45 %50
+
+%45 = OpLabel
+; This value is hoisted
+%600 = OpCopyObject %float %float_50
+OpBranch %50
+
+%50 = OpLabel
+OpBranch %90
+
+%80 = OpLabel ; unreachable continue target
+%82 = OpConvertFToU %uint %600
+OpBranch %30 ; backedge
+
+%90 = OpLabel
+OpReturn
+OpFunctionEnd
+
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
+ auto fe = p->function_emitter(100);
+ EXPECT_TRUE(fe.EmitBody()) << p->error();
+ EXPECT_THAT(ToString(p->builder(), fe.ast_body()), HasSubstr(R"(VariableConst{
+ x_82
+ none
+ __u32
+ {
+ TypeConstructor[not set]{
+ __u32
+ Identifier[not set]{x_600}
+ }
+ }
+ })"));
+}
+
// TODO(dneto): OpSConvert // only if multiple widths
// TODO(dneto): OpUConvert // only if multiple widths
// TODO(dneto): OpFConvert // only if multiple widths
diff --git a/src/reader/spirv/parser_type.cc b/src/reader/spirv/parser_type.cc
index b6c835c..a2db682 100644
--- a/src/reader/spirv/parser_type.cc
+++ b/src/reader/spirv/parser_type.cc
@@ -336,6 +336,14 @@
return type;
}
+const Type* Type::UnwrapRef() const {
+ const Type* type = this;
+ while (auto* ptr = type->As<Reference>()) {
+ type = ptr->type;
+ }
+ return type;
+}
+
const Type* Type::UnwrapAlias() const {
const Type* type = this;
while (auto* alias = type->As<Alias>()) {
diff --git a/src/reader/spirv/parser_type.h b/src/reader/spirv/parser_type.h
index 51673d5..c01c4e1 100644
--- a/src/reader/spirv/parser_type.h
+++ b/src/reader/spirv/parser_type.h
@@ -49,6 +49,10 @@
/// @returns the inner most store type if this is a pointer, `this` otherwise
const Type* UnwrapPtr() const;
+ /// @returns the inner most store type if this is a reference, `this`
+ /// otherwise
+ const Type* UnwrapRef() const;
+
/// @returns the inner most aliased type if this is an alias, `this` otherwise
const Type* UnwrapAlias() const;