diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 61dae65ac4..97d1f18796 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -4475,6 +4475,7 @@ TypedExpression FunctionEmitter::MakeNumericConversion( 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 @@ TypedExpression FunctionEmitter::MakeNumericConversion( 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 @@ TypedExpression FunctionEmitter::MakeNumericConversion( ast::ExpressionList params; params.push_back(arg_expr.expr); - TypedExpression result{ - expr_type, create( - Source{}, expr_type->Build(builder_), std::move(params))}; + TypedExpression result{expr_type, + create( + GetSourceForInst(inst), expr_type->Build(builder_), + std::move(params))}; if (requested_type == expr_type) { return result; } - return {requested_type, - create( - Source{}, requested_type->Build(builder_), result.expr)}; + return {requested_type, create( + 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 8dc4999ac8..6d77b2704a 100644 --- a/src/reader/spirv/function_conversion_test.cc +++ b/src/reader/spirv/function_conversion_test.cc @@ -343,8 +343,9 @@ TEST_F(SpvUnaryConversionTest, ConvertUToF_Scalar_BadArgType) { 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 @@ TEST_F(SpvUnaryConversionTest, ConvertUToF_Vector_BadArgType) { 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 @@ TEST_F(SpvUnaryConversionTest, ConvertFToS_Scalar_BadArgType) { 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 @@ TEST_F(SpvUnaryConversionTest, ConvertFToS_Vector_BadArgType) { 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 @@ TEST_F(SpvUnaryConversionTest, ConvertFToU_Scalar_BadArgType) { 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 @@ TEST_F(SpvUnaryConversionTest, ConvertFToU_Vector_BadArgType) { 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_Vector_ToUnsigned) { })")); } +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 b6c835c688..a2db6821fe 100644 --- a/src/reader/spirv/parser_type.cc +++ b/src/reader/spirv/parser_type.cc @@ -336,6 +336,14 @@ const Type* Type::UnwrapPtr() const { return type; } +const Type* Type::UnwrapRef() const { + const Type* type = this; + while (auto* ptr = type->As()) { + type = ptr->type; + } + return type; +} + const Type* Type::UnwrapAlias() const { const Type* type = this; while (auto* alias = type->As()) { diff --git a/src/reader/spirv/parser_type.h b/src/reader/spirv/parser_type.h index 51673d592c..c01c4e1a7e 100644 --- a/src/reader/spirv/parser_type.h +++ b/src/reader/spirv/parser_type.h @@ -49,6 +49,10 @@ class Type : public Castable { /// @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;