From d5f4ea22f03abbec99d5cb785ff5bc1e40015bac Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 27 May 2021 20:46:46 +0000 Subject: [PATCH] wsgl-writer: emit inf, nan, subnormal as hex float Signed zeros are emitted. Subormal numbers are emitted as hex float. Handling Inf and NaN is unresolved in the spec. See https://github.com/gpuweb/gpuweb/issues/1769 NaN tests are disabled, due to platform dependence. Windows x86-64 seems to always set the high mantissa bit on NaNs. Fixed: tint:76 Change-Id: I06c69b93b04c869a63ec2f574022996acc7a6dbe Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/52321 Commit-Queue: David Neto Auto-Submit: David Neto Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/CMakeLists.txt | 1 + src/writer/float_to_string.cc | 105 +++++++++++++ src/writer/float_to_string.h | 5 + src/writer/float_to_string_test.cc | 147 ++++++++++++++++++ src/writer/wgsl/generator_impl.cc | 2 +- .../wgsl/generator_impl_literal_test.cc | 132 ++++++++++++++++ test/BUILD.gn | 3 +- 7 files changed, 393 insertions(+), 2 deletions(-) create mode 100644 src/writer/wgsl/generator_impl_literal_test.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bed580761b..a1c6a05433 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -789,6 +789,7 @@ if(${TINT_BUILD_TESTS}) writer/wgsl/generator_impl_identifier_test.cc writer/wgsl/generator_impl_if_test.cc writer/wgsl/generator_impl_loop_test.cc + writer/wgsl/generator_impl_literal_test.cc writer/wgsl/generator_impl_member_accessor_test.cc writer/wgsl/generator_impl_return_test.cc writer/wgsl/generator_impl_switch_test.cc diff --git a/src/writer/float_to_string.cc b/src/writer/float_to_string.cc index b179ed05bc..1692f26451 100644 --- a/src/writer/float_to_string.cc +++ b/src/writer/float_to_string.cc @@ -14,9 +14,14 @@ #include "src/writer/float_to_string.h" +#include +#include +#include #include #include +#include "src/debug.h" + namespace tint { namespace writer { @@ -33,5 +38,105 @@ std::string FloatToString(float f) { return str; } +std::string FloatToBitPreservingString(float f) { + // For the NaN case, avoid handling the number as a floating point value. + // Some machines will modify the top bit in the mantissa of a NaN. + + std::stringstream ss; + + uint32_t float_bits = 0u; + std::memcpy(&float_bits, &f, sizeof(float_bits)); + + // Handle the sign. + const uint32_t kSignMask = 1u << 31; + if (float_bits & kSignMask) { + // If `f` is -0.0 print -0.0. + ss << '-'; + // Strip sign bit. + float_bits = float_bits & (~kSignMask); + } + + switch (std::fpclassify(f)) { + case FP_ZERO: + case FP_NORMAL: + std::memcpy(&f, &float_bits, sizeof(float_bits)); + ss << FloatToString(f); + break; + + default: { + // Infinity, NaN, and Subnormal + // TODO(dneto): It's unclear how Infinity and NaN should be handled. + // See https://github.com/gpuweb/gpuweb/issues/1769 + + // std::hexfloat prints 'nan' and 'inf' instead of an + // explicit representation like we want. Split it out + // manually. + const int kExponentBias = 127; + const int kExponentMask = 0x7f800000; + const int kMantissaMask = 0x007fffff; + const int kMantissaBits = 23; + + int mantissaNibbles = (kMantissaBits + 3) / 4; + + const int biased_exponent = + static_cast((float_bits & kExponentMask) >> kMantissaBits); + int exponent = biased_exponent - kExponentBias; + uint32_t mantissa = float_bits & kMantissaMask; + + ss << "0x"; + + if (exponent == 128) { + if (mantissa == 0) { + // Infinity case. + ss << "1p+128"; + } else { + // NaN case. + // Emit the mantissa bits as if they are left-justified after the + // binary point. This is what SPIRV-Tools hex float emitter does, + // and it's a justifiable choice independent of the bit width + // of the mantissa. + mantissa <<= (4 - (kMantissaBits % 4)); + // Remove trailing zeroes, for tidyness. + while (0 == (0xf & mantissa)) { + mantissa >>= 4; + mantissaNibbles--; + } + ss << "1." << std::hex << std::setfill('0') + << std::setw(mantissaNibbles) << mantissa << "p+128"; + } + } else { + // Subnormal, and not zero. + TINT_ASSERT(mantissa != 0); + const int kTopBit = (1 << kMantissaBits); + + // Shift left until we get 1.x + while (0 == (kTopBit & mantissa)) { + mantissa <<= 1; + exponent--; + } + // Emit the leading 1, and remove it from the mantissa. + ss << "1"; + mantissa = mantissa ^ kTopBit; + mantissa <<= 1; + exponent++; + + // Emit the fractional part. + if (mantissa) { + // Remove trailing zeroes, for tidyness + while (0 == (0xf & mantissa)) { + mantissa >>= 4; + mantissaNibbles--; + } + ss << "." << std::hex << std::setfill('0') + << std::setw(mantissaNibbles) << mantissa; + } + // Emit the exponent + ss << "p" << std::showpos << std::dec << exponent; + } + } + } + return ss.str(); +} + } // namespace writer } // namespace tint diff --git a/src/writer/float_to_string.h b/src/writer/float_to_string.h index cd29134ea0..9f385dbb44 100644 --- a/src/writer/float_to_string.h +++ b/src/writer/float_to_string.h @@ -28,6 +28,11 @@ namespace writer { /// @return the float f formatted to a string std::string FloatToString(float f); +/// Converts the float `f` to a string, using hex float notation for infinities, +/// NaNs, or subnormal numbers. Otherwise behaves as FloatToString. +/// @return the float f formatted to a string +std::string FloatToBitPreservingString(float f); + } // namespace writer } // namespace tint diff --git a/src/writer/float_to_string_test.cc b/src/writer/float_to_string_test.cc index dda45544bd..4a0f64cea4 100644 --- a/src/writer/float_to_string_test.cc +++ b/src/writer/float_to_string_test.cc @@ -14,6 +14,8 @@ #include "src/writer/float_to_string.h" +#include +#include #include #include "gtest/gtest.h" @@ -22,6 +24,25 @@ namespace tint { namespace writer { namespace { +// Makes an IEEE 754 binary32 floating point number with +// - 0 sign if sign is 0, 1 otherwise +// - 'exponent_bits' is placed in the exponent space. +// So, the exponent bias must already be included. +float MakeFloat(int sign, int biased_exponent, int mantissa) { + const uint32_t sign_bit = sign ? 0x80000000u : 0u; + // The binary32 exponent is 8 bits, just below the sign. + const uint32_t exponent_bits = (biased_exponent & 0xffu) << 23; + // The mantissa is the bottom 23 bits. + const uint32_t mantissa_bits = (mantissa & 0x7fffffu); + + uint32_t bits = sign_bit | exponent_bits | mantissa_bits; + float result = 0.0f; + static_assert(sizeof(result) == sizeof(bits), + "expected float and uint32_t to be the same size"); + std::memcpy(&result, &bits, sizeof(bits)); + return result; +} + TEST(FloatToStringTest, Zero) { EXPECT_EQ(FloatToString(0.0f), "0.0"); } @@ -67,6 +88,132 @@ TEST(FloatToStringTest, Lowest) { "-340282346638528859811704183484516925440.0"); } +// FloatToBitPreservingString +// +// First replicate the tests for FloatToString + +TEST(FloatToBitPreservingStringTest, Zero) { + EXPECT_EQ(FloatToBitPreservingString(0.0f), "0.0"); +} + +TEST(FloatToBitPreservingStringTest, One) { + EXPECT_EQ(FloatToBitPreservingString(1.0f), "1.0"); +} + +TEST(FloatToBitPreservingStringTest, MinusOne) { + EXPECT_EQ(FloatToBitPreservingString(-1.0f), "-1.0"); +} + +TEST(FloatToBitPreservingStringTest, Billion) { + EXPECT_EQ(FloatToBitPreservingString(1e9f), "1000000000.0"); +} + +TEST(FloatToBitPreservingStringTest, Small) { + EXPECT_NE(FloatToBitPreservingString(std::numeric_limits::epsilon()), + "0.0"); +} + +TEST(FloatToBitPreservingStringTest, Highest) { + const auto highest = std::numeric_limits::max(); + const auto expected_highest = 340282346638528859811704183484516925440.0f; + if (highest < expected_highest || highest > expected_highest) { + GTEST_SKIP() << "std::numeric_limits::max() is not as expected for " + "this target"; + } + EXPECT_EQ(FloatToBitPreservingString(std::numeric_limits::max()), + "340282346638528859811704183484516925440.0"); +} + +TEST(FloatToBitPreservingStringTest, Lowest) { + // Some compilers complain if you test floating point numbers for equality. + // So say it via two inequalities. + const auto lowest = std::numeric_limits::lowest(); + const auto expected_lowest = -340282346638528859811704183484516925440.0f; + if (lowest < expected_lowest || lowest > expected_lowest) { + GTEST_SKIP() + << "std::numeric_limits::lowest() is not as expected for " + "this target"; + } + EXPECT_EQ(FloatToBitPreservingString(std::numeric_limits::lowest()), + "-340282346638528859811704183484516925440.0"); +} + +// Special cases for bit-preserving output. + +TEST(FloatToBitPreservingStringTest, NegativeZero) { + EXPECT_EQ(FloatToBitPreservingString(std::copysign(0.0f, -5.0f)), "-0.0"); +} + +TEST(FloatToBitPreservingStringTest, ZeroAsBits) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 0)), "0.0"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 0, 0)), "-0.0"); +} + +TEST(FloatToBitPreservingStringTest, OneBits) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 127, 0)), "1.0"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 127, 0)), "-1.0"); +} + +TEST(FloatToBitPreservingStringTest, SmallestDenormal) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 1)), "0x1p-149"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 0, 1)), "-0x1p-149"); +} + +TEST(FloatToBitPreservingStringTest, BiggerDenormal) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 2)), "0x1p-148"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 0, 2)), "-0x1p-148"); +} + +TEST(FloatToBitPreservingStringTest, LargestDenormal) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 0x7fffff)), + "0x1.fffffcp-127"); +} + +TEST(FloatToBitPreservingStringTest, Subnormal_cafebe) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 0xcafebe)), + "0x1.2bfaf8p-127"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 0, 0xcafebe)), + "-0x1.2bfaf8p-127"); +} + +TEST(FloatToBitPreservingStringTest, Subnormal_aaaaa) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 0, 0xaaaaa)), + "0x1.55554p-130"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 0, 0xaaaaa)), + "-0x1.55554p-130"); +} + +TEST(FloatToBitPreservingStringTest, Infinity) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 255, 0)), "0x1p+128"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 255, 0)), "-0x1p+128"); +} + +// TODO(dneto): It's unclear how Infinity and NaN should be handled. +// https://github.com/gpuweb/gpuweb/issues/1769 +// Windows x86-64 sets the high mantissa bit on NaNs. +// Disable NaN tests for now. + +TEST(FloatToBitPreservingStringTest, DISABLED_NaN_MsbOnly) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 255, 0x400000)), + "0x1.8p+128"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 255, 0x400000)), + "-0x1.8p+128"); +} + +TEST(FloatToBitPreservingStringTest, DISABLED_NaN_LsbOnly) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 255, 0x1)), + "0x1.000002p+128"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 255, 0x1)), + "-0x1.000002p+128"); +} + +TEST(FloatToBitPreservingStringTest, DISABLED_NaN_NonMsb) { + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(0, 255, 0x20101f)), + "0x1.40203ep+128"); + EXPECT_EQ(FloatToBitPreservingString(MakeFloat(1, 255, 0x20101f)), + "-0x1.40203ep+128"); +} + } // namespace } // namespace writer } // namespace tint diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index ef96c538f2..17b83598d6 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -253,7 +253,7 @@ bool GeneratorImpl::EmitLiteral(ast::Literal* lit) { if (auto* bl = lit->As()) { out_ << (bl->IsTrue() ? "true" : "false"); } else if (auto* fl = lit->As()) { - out_ << FloatToString(fl->value()); + out_ << FloatToBitPreservingString(fl->value()); } else if (auto* sl = lit->As()) { out_ << sl->value(); } else if (auto* ul = lit->As()) { diff --git a/src/writer/wgsl/generator_impl_literal_test.cc b/src/writer/wgsl/generator_impl_literal_test.cc new file mode 100644 index 0000000000..6ded4c8d40 --- /dev/null +++ b/src/writer/wgsl/generator_impl_literal_test.cc @@ -0,0 +1,132 @@ +// Copyright 2020 The Tint Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "src/ast/override_decoration.h" +#include "src/writer/wgsl/test_helper.h" + +namespace tint { +namespace writer { +namespace wgsl { +namespace { + +// Makes an IEEE 754 binary32 floating point number with +// - 0 sign if sign is 0, 1 otherwise +// - 'exponent_bits' is placed in the exponent space. +// So, the exponent bias must already be included. +float MakeFloat(int sign, int biased_exponent, int mantissa) { + const uint32_t sign_bit = sign ? 0x80000000u : 0u; + // The binary32 exponent is 8 bits, just below the sign. + const uint32_t exponent_bits = (biased_exponent & 0xffu) << 23; + // The mantissa is the bottom 23 bits. + const uint32_t mantissa_bits = (mantissa & 0x7fffffu); + + uint32_t bits = sign_bit | exponent_bits | mantissa_bits; + float result = 0.0f; + static_assert(sizeof(result) == sizeof(bits), + "expected float and uint32_t to be the same size"); + std::memcpy(&result, &bits, sizeof(bits)); + return result; +} + +struct FloatData { + float value; + std::string expected; +}; +inline std::ostream& operator<<(std::ostream& out, FloatData data) { + out << "{" << data.value << "," << data.expected << "}"; + return out; +} + +using WgslGenerator_FloatLiteralTest = TestParamHelper; + +TEST_P(WgslGenerator_FloatLiteralTest, Emit) { + auto* v = Expr(GetParam().value); + + SetResolveOnBuild(false); + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.EmitScalarConstructor(v)) << gen.error(); + EXPECT_EQ(gen.result(), GetParam().expected); +} + +INSTANTIATE_TEST_SUITE_P(Zero, + WgslGenerator_FloatLiteralTest, + ::testing::ValuesIn(std::vector{ + {0.0f, "0.0"}, + {MakeFloat(0, 0, 0), "0.0"}, + {MakeFloat(1, 0, 0), "-0.0"}})); + +INSTANTIATE_TEST_SUITE_P(Normal, + WgslGenerator_FloatLiteralTest, + ::testing::ValuesIn(std::vector{ + {1.0f, "1.0"}, + {-1.0f, "-1.0"}, + {101.375, "101.375"}})); + +INSTANTIATE_TEST_SUITE_P( + Subnormal, + WgslGenerator_FloatLiteralTest, + ::testing::ValuesIn(std::vector{ + {MakeFloat(0, 0, 1), "0x1p-149"}, // Smallest + {MakeFloat(1, 0, 1), "-0x1p-149"}, + {MakeFloat(0, 0, 2), "0x1p-148"}, + {MakeFloat(1, 0, 2), "-0x1p-148"}, + {MakeFloat(0, 0, 0x7fffff), "0x1.fffffcp-127"}, // Largest + {MakeFloat(1, 0, 0x7fffff), "-0x1.fffffcp-127"}, // Largest + {MakeFloat(0, 0, 0xcafebe), "0x1.2bfaf8p-127"}, // Scattered bits + {MakeFloat(1, 0, 0xcafebe), "-0x1.2bfaf8p-127"}, // Scattered bits + {MakeFloat(0, 0, 0xaaaaa), "0x1.55554p-130"}, // Scattered bits + {MakeFloat(1, 0, 0xaaaaa), "-0x1.55554p-130"}, // Scattered bits + })); + +INSTANTIATE_TEST_SUITE_P(Infinity, + WgslGenerator_FloatLiteralTest, + ::testing::ValuesIn(std::vector{ + {MakeFloat(0, 255, 0), "0x1p+128"}, + {MakeFloat(1, 255, 0), "-0x1p+128"}})); + +INSTANTIATE_TEST_SUITE_P( + // TODO(dneto): It's unclear how Infinity and NaN should be handled. + // https://github.com/gpuweb/gpuweb/issues/1769 + // This test fails on Windows x86-64 because the machine sets the high + // mantissa bit on NaNs. + DISABLED_NaN, + // In the NaN case, the top bit in the mantissa is often used to encode + // whether the NaN is signalling or quiet, but no agreement between + // different machine architectures on whether 1 means signalling or + // if 1 means quiet. + WgslGenerator_FloatLiteralTest, + ::testing::ValuesIn(std::vector{ + // LSB only. Smallest mantissa. + {MakeFloat(0, 255, 1), "0x1.000002p+128"}, // Smallest mantissa + {MakeFloat(1, 255, 1), "-0x1.000002p+128"}, + // MSB only. + {MakeFloat(0, 255, 0x400000), "0x1.8p+128"}, + {MakeFloat(1, 255, 0x400000), "-0x1.8p+128"}, + // All 1s in the mantissa. + {MakeFloat(0, 255, 0x7fffff), "0x1.fffffep+128"}, + {MakeFloat(1, 255, 0x7fffff), "-0x1.fffffep+128"}, + // Scattered bits, with 0 in top mantissa bit. + {MakeFloat(0, 255, 0x20101f), "0x1.40203ep+128"}, + {MakeFloat(1, 255, 0x20101f), "-0x1.40203ep+128"}, + // Scattered bits, with 1 in top mantissa bit. + {MakeFloat(0, 255, 0x40101f), "0x1.80203ep+128"}, + {MakeFloat(1, 255, 0x40101f), "-0x1.80203ep+128"}})); + +} // namespace +} // namespace wgsl +} // namespace writer +} // namespace tint diff --git a/test/BUILD.gn b/test/BUILD.gn index 1ea5ae7810..30cc5dba25 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -258,9 +258,9 @@ tint_unittests_source_set("tint_unittests_core_src") { "../src/resolver/intrinsic_test.cc", "../src/resolver/is_host_shareable_test.cc", "../src/resolver/is_storeable_test.cc", + "../src/resolver/pipeline_overridable_constant_test.cc", "../src/resolver/ptr_ref_test.cc", "../src/resolver/ptr_ref_validation_test.cc", - "../src/resolver/pipeline_overridable_constant_test.cc", "../src/resolver/resolver_test.cc", "../src/resolver/resolver_test_helper.cc", "../src/resolver/resolver_test_helper.h", @@ -511,6 +511,7 @@ tint_unittests_source_set("tint_unittests_wgsl_writer_src") { "../src/writer/wgsl/generator_impl_global_decl_test.cc", "../src/writer/wgsl/generator_impl_identifier_test.cc", "../src/writer/wgsl/generator_impl_if_test.cc", + "../src/writer/wgsl/generator_impl_literal_test.cc", "../src/writer/wgsl/generator_impl_loop_test.cc", "../src/writer/wgsl/generator_impl_member_accessor_test.cc", "../src/writer/wgsl/generator_impl_return_test.cc",