From 9d4c24fa5e5e95f40ba3aec8a5b221aa74deeb3c Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 29 Jul 2021 17:54:35 +0000 Subject: [PATCH] spirv-reader: ldexp second argument must be signed Fixed: tint:1079 Change-Id: I628b81d95201474c6d1c584eafacd125448de30b Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/60240 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/reader/spirv/function.cc | 14 ++ .../spirv/function_glsl_std_450_test.cc | 145 +++++++++--------- 2 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 727ad83a72..b4aeaeccda 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -3986,6 +3986,20 @@ TypedExpression FunctionEmitter::EmitGlslStd450ExtInst( const spvtools::opt::Instruction& inst) { const auto ext_opcode = inst.GetSingleWordInOperand(1); + if (ext_opcode == GLSLstd450Ldexp) { + // WGSL requires the second argument to be signed. + // Use a type constructor to convert it, which is the same as a bitcast. + // If the value would go from very large positive to negative, then the + // original result would have been infinity. And since WGSL + // implementations may assume that infinities are not present, then we + // don't have to worry about that case. + auto e1 = MakeOperand(inst, 2); + auto e2 = ToSignedIfUnsigned(MakeOperand(inst, 3)); + + return {e1.type, builder_.Call(Source{}, "ldexp", + ast::ExpressionList{e1.expr, e2.expr})}; + } + auto* result_type = parser_impl_.ConvertType(inst.type_id()); if (result_type->IsScalar()) { diff --git a/src/reader/spirv/function_glsl_std_450_test.cc b/src/reader/spirv/function_glsl_std_450_test.cc index 05ad700cdc..2159ef0bd3 100644 --- a/src/reader/spirv/function_glsl_std_450_test.cc +++ b/src/reader/spirv/function_glsl_std_450_test.cc @@ -158,8 +158,6 @@ using SpvParserTest_GlslStd450_Floating_FloatingFloatingFloating = SpvParserTestBase<::testing::TestWithParam>; using SpvParserTest_GlslStd450_Floating_FloatingInting = SpvParserTestBase<::testing::TestWithParam>; -using SpvParserTest_GlslStd450_Floating_FloatingUinting = - SpvParserTestBase<::testing::TestWithParam>; using SpvParserTest_GlslStd450_Float3_Float3Float3 = SpvParserTestBase<::testing::TestWithParam>; @@ -503,73 +501,6 @@ TEST_P(SpvParserTest_GlslStd450_Floating_FloatingFloatingFloating, Vector) { << body; } -TEST_P(SpvParserTest_GlslStd450_Floating_FloatingUinting, Scalar) { - const auto assembly = Preamble() + R"( - %1 = OpExtInst %float %glsl )" + - GetParam().opcode + R"( %f1 %u1 - OpReturn - OpFunctionEnd - )"; - auto p = parser(test::Assemble(assembly)); - ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); - auto fe = p->function_emitter(100); - EXPECT_TRUE(fe.EmitBody()) << p->error(); - const auto body = ToString(p->builder(), fe.ast_body()); - EXPECT_THAT(body, HasSubstr(R"( - VariableConst{ - x_1 - none - undefined - __f32 - { - Call[not set]{ - Identifier[not set]{)" + - GetParam().wgsl_func + - R"(} - ( - Identifier[not set]{f1} - Identifier[not set]{u1} - ) - } - } - })")) - << body; -} - -TEST_P(SpvParserTest_GlslStd450_Floating_FloatingUinting, Vector) { - const auto assembly = Preamble() + R"( - %1 = OpExtInst %v2float %glsl )" + - GetParam().opcode + - R"( %v2f1 %v2u1 - OpReturn - OpFunctionEnd - )"; - auto p = parser(test::Assemble(assembly)); - ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); - auto fe = p->function_emitter(100); - EXPECT_TRUE(fe.EmitBody()) << p->error(); - const auto body = ToString(p->builder(), fe.ast_body()); - EXPECT_THAT(body, HasSubstr(R"( - VariableConst{ - x_1 - none - undefined - __vec_2__f32 - { - Call[not set]{ - Identifier[not set]{)" + - GetParam().wgsl_func + - R"(} - ( - Identifier[not set]{v2f1} - Identifier[not set]{v2u1} - ) - } - } - })")) - << body; -} - TEST_P(SpvParserTest_GlslStd450_Floating_FloatingInting, Scalar) { const auto assembly = Preamble() + R"( %1 = OpExtInst %float %glsl )" + @@ -720,13 +651,10 @@ INSTANTIATE_TEST_SUITE_P(Samples, {"Step", "step"}, })); -INSTANTIATE_TEST_SUITE_P(Samples, - SpvParserTest_GlslStd450_Floating_FloatingUinting, - ::testing::Values(GlslStd450Case{"Ldexp", "ldexp"})); - INSTANTIATE_TEST_SUITE_P(Samples, SpvParserTest_GlslStd450_Floating_FloatingInting, ::testing::Values(GlslStd450Case{"Ldexp", "ldexp"})); +// For ldexp with unsigned second argument, see below. INSTANTIATE_TEST_SUITE_P(Samples, SpvParserTest_GlslStd450_Float3_Float3Float3, @@ -2187,6 +2115,77 @@ TEST_F(SpvParserTest, GlslStd450_Radians_Vector) { EXPECT_THAT(body, HasSubstr(expected)) << body; } +// For ldexp with signed second argument, see above. +TEST_F(SpvParserTest, GlslStd450_Ldexp_Scalar_Float_Uint) { + const auto assembly = Preamble() + R"( + %1 = OpExtInst %float %glsl Ldexp %f1 %u1 + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + auto fe = p->function_emitter(100); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + const auto body = ToString(p->builder(), fe.ast_body()); + const auto* expected = R"(VariableDeclStatement{ + VariableConst{ + x_1 + none + undefined + __f32 + { + Call[not set]{ + Identifier[not set]{ldexp} + ( + Identifier[not set]{f1} + TypeConstructor[not set]{ + __i32 + Identifier[not set]{u1} + } + ) + } + } + } +})"; + + EXPECT_THAT(body, HasSubstr(expected)) << body; +} + +TEST_F(SpvParserTest, GlslStd450_Ldexp_Vector_Floatvec_Uintvec) { + const auto assembly = Preamble() + R"( + %1 = OpExtInst %v2float %glsl Ldexp %v2f1 %v2u1 + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + auto fe = p->function_emitter(100); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + const auto body = ToString(p->builder(), fe.ast_body()); + const auto* expected = R"(VariableDeclStatement{ + VariableConst{ + x_1 + none + undefined + __vec_2__f32 + { + Call[not set]{ + Identifier[not set]{ldexp} + ( + Identifier[not set]{v2f1} + TypeConstructor[not set]{ + __vec_2__i32 + Identifier[not set]{v2u1} + } + ) + } + } + } +})"; + + EXPECT_THAT(body, HasSubstr(expected)) << body; +} + } // namespace } // namespace spirv } // namespace reader