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;