From 229b90f6f4e2756748a6dcac2c5f5fb7375210bc Mon Sep 17 00:00:00 2001 From: David Neto Date: Fri, 6 Nov 2015 11:23:57 -0500 Subject: [PATCH] Print OpConstant values according to type. Zero and normal floating point values are printed with enough enough digits to reproduce all the bits exactly. Other float values (subnormal, infinity, and NaN) are printed as hex floats. Fix a binary parse bug: Count partially filled words in a typed literal number operand. TODO: Assembler support for hex numbers, and therefore reading infinities and NaNs. --- include/util/hex_float.h | 24 ++++++- source/binary.cpp | 2 +- source/disassemble.cpp | 41 +++++++++-- test/BinaryToText.cpp | 5 +- test/HexFloat.cpp | 157 ++++++++++++++++++++++++++++++++++++++--- test/TestFixture.h | 2 +- test/TextToBinary.Constant.cpp | 11 ++- test/TextToBinary.cpp | 18 ++--- 8 files changed, 226 insertions(+), 34 deletions(-) diff --git a/include/util/hex_float.h b/include/util/hex_float.h index d60c3f5..26dade5 100644 --- a/include/util/hex_float.h +++ b/include/util/hex_float.h @@ -97,7 +97,7 @@ bool operator==(const FloatProxy& first, const FloatProxy& second) { return first.data() == second.data(); } -// Convenience to read the value as a normal float. +// Reads a FloatProxy value as a normal float from a stream. template std::istream& operator>>(std::istream& is, FloatProxy& value) { T float_val; @@ -520,6 +520,28 @@ std::istream& operator>>(std::istream& is, HexFloat& value) { return is; } + +// Writes a FloatProxy value to a stream. +// Zero and normal numbers are printed in the usual notation, but with +// enough digits to fully reproduce the value. Other values (subnormal, +// NaN, and infinity) are printed as a hex float. +template +std::ostream& operator<<(std::ostream& os, const FloatProxy& value) { + auto float_val = value.getAsFloat(); + switch (std::fpclassify(float_val)) { + case FP_ZERO: + case FP_NORMAL: { + auto saved_precision = os.precision(); + os.precision(std::numeric_limits::digits10); + os << float_val; + os.precision(saved_precision); + } break; + default: + os << HexFloat>(value); + break; + } + return os; +} } #endif // _LIBSPIRV_UTIL_HEX_FLOAT_H_ diff --git a/source/binary.cpp b/source/binary.cpp index b45123b..f3df4ad 100644 --- a/source/binary.cpp +++ b/source/binary.cpp @@ -628,7 +628,7 @@ spv_result_t Parser::setNumericTypeInfoForType( parsed_operand->number_kind = info.type; parsed_operand->number_bit_width = info.bit_width; - parsed_operand->num_words = info.bit_width / 32; + parsed_operand->num_words = (info.bit_width + 31) / 32; // Round up return SPV_SUCCESS; } diff --git a/source/disassemble.cpp b/source/disassemble.cpp index ab75e2e..de720d0 100644 --- a/source/disassemble.cpp +++ b/source/disassemble.cpp @@ -39,6 +39,7 @@ #include "libspirv/libspirv.h" #include "opcode.h" #include "print.h" +#include "util/hex_float.h" namespace { @@ -185,12 +186,40 @@ void Disassembler::EmitOperand(const spv_parsed_instruction_t& inst, case SPV_OPERAND_TYPE_LITERAL_INTEGER: case SPV_OPERAND_TYPE_TYPED_LITERAL_NUMBER: { SetRed(); - // TODO(dneto): Emit values according to type. - if (operand.num_words == 1) - stream_ << word; - else if (operand.num_words == 2) - stream_ << spvFixDoubleWord(words_[index], words_[index + 1], endian_); - else { + if (operand.num_words == 1) { + switch (operand.number_kind) { + case SPV_NUMBER_SIGNED_INT: + stream_ << int32_t(word); + break; + case SPV_NUMBER_UNSIGNED_INT: + stream_ << uint32_t(word); + break; + case SPV_NUMBER_FLOATING: + // Assume only 32-bit floats. + // TODO(dneto): Handle 16-bit floats also. + stream_ << spvutils::FloatProxy(word); + break; + default: + assert(false && "Unreachable"); + } + } else if (operand.num_words == 2) { + uint64_t bits = + spvFixDoubleWord(words_[index], words_[index + 1], endian_); + switch (operand.number_kind) { + case SPV_NUMBER_SIGNED_INT: + stream_ << int64_t(bits); + break; + case SPV_NUMBER_UNSIGNED_INT: + stream_ << uint64_t(bits); + break; + case SPV_NUMBER_FLOATING: + // Assume only 64-bit floats. + stream_ << spvutils::FloatProxy(bits); + break; + default: + assert(false && "Unreachable"); + } + } else { // TODO(dneto): Support more than 64-bits at a time. assert("Unhandled"); } diff --git a/test/BinaryToText.cpp b/test/BinaryToText.cpp index dea3ab1..eb1cd1f 100644 --- a/test/BinaryToText.cpp +++ b/test/BinaryToText.cpp @@ -350,7 +350,6 @@ TEST(BinaryToTextSmall, LiteralDouble) { spv_binary binary; spv_diagnostic diagnostic = nullptr; - // Pi: 3.1415926535897930 => 0x400921fb54442d18 => 4614256656552045848 AutoText input( "%1 = OpTypeFloat 64\n%2 = OpSpecConstant %1 3.1415926535897930"); spv_result_t error = @@ -373,9 +372,9 @@ TEST(BinaryToTextSmall, LiteralDouble) { ; Bound: 3 ; Schema: 0 %1 = OpTypeFloat 64 -%2 = OpSpecConstant %1 4614256656552045848 +%2 = OpSpecConstant %1 3.14159265358979 )"; - EXPECT_EQ(output, text->str); + EXPECT_EQ(output, text->str) << text->str; spvTextDestroy(text); } diff --git a/test/HexFloat.cpp b/test/HexFloat.cpp index c578722..10c5fb7 100644 --- a/test/HexFloat.cpp +++ b/test/HexFloat.cpp @@ -24,20 +24,24 @@ // TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE // MATERIALS OR THE USE OR OTHER DEALINGS IN THE MATERIALS. -#include "UnitSPIRV.h" -#include "util/hex_float.h" - -#include #include +#include #include #include #include +#include +#include "UnitSPIRV.h" +#include "util/hex_float.h" + namespace { using ::testing::Eq; using spvutils::BitwiseCast; using spvutils::FloatProxy; +// In this file "encode" means converting a number into a string, +// and "decode" means converting a string into a number. + using HexFloatTest = ::testing::TestWithParam, std::string>>; using DecodeHexFloatTest = @@ -49,7 +53,7 @@ using DecodeHexDoubleTest = // Hex-encodes a float value. template -std::string Encode(const T& value) { +std::string EncodeViaHexFloat(const T& value) { std::stringstream ss; ss << spvutils::HexFloat(value); return ss.str(); @@ -59,18 +63,18 @@ std::string Encode(const T& value) { // types. TEST_P(HexFloatTest, EncodeCorrectly) { - EXPECT_THAT(Encode(GetParam().first), Eq(GetParam().second)); + EXPECT_THAT(EncodeViaHexFloat(GetParam().first), Eq(GetParam().second)); } TEST_P(HexDoubleTest, EncodeCorrectly) { - EXPECT_THAT(Encode(GetParam().first), Eq(GetParam().second)); + EXPECT_THAT(EncodeViaHexFloat(GetParam().first), Eq(GetParam().second)); } // Decodes a hex-float string. template FloatProxy Decode(const std::string& str) { spvutils::HexFloat> decoded(0.f); - std::stringstream(str) >> decoded; + EXPECT_TRUE((std::stringstream(str) >> decoded).eof()); return decoded.value(); } @@ -387,5 +391,142 @@ TEST(FloatProxy, Negation) { Eq(std::numeric_limits::infinity())); } +// Test conversion of FloatProxy values to strings. +// +// In previous cases, we always wrapped the FloatProxy value in a HexFloat +// before conversion to a string. In the following cases, the FloatProxy +// decides for itself whether to print as a regular number or as a hex float. + +using FloatProxyFloatTest = + ::testing::TestWithParam, std::string>>; +using FloatProxyDoubleTest = + ::testing::TestWithParam, std::string>>; + +// Converts a float value to a string via a FloatProxy. +template +std::string EncodeViaFloatProxy(const T& value) { + std::stringstream ss; + ss << value; + return ss.str(); +} + +// Converts a floating point string so that the exponent prefix +// is 'e', and the exponent value does not have leading zeros. +// The Microsoft runtime library likes to write things like "2.5E+010". +// Convert that to "2.5e+10". +// We don't care what happens to strings that are not floating point +// strings. +std::string NormalizeExponentInFloatString(std::string in) { + std::string result; + // Reserve one spot for the terminating null, even when the sscanf fails. + char prefix[in.size() + 1]; + char e; + char plus_or_minus; + int exponent; // in base 10 + if ((4 == std::sscanf(in.c_str(), "%[-+.0123456789]%c%c%d", prefix, &e, + &plus_or_minus, &exponent)) && + (e == 'e' || e == 'E') && + (plus_or_minus == '-' || plus_or_minus == '+')) { + // It looks like a floating point value with exponent. + std::stringstream out; + out << prefix << 'e' << plus_or_minus << exponent; + result = out.str(); + } else { + result = in; + } + return result; +} + +TEST(NormalizeFloat, Sample) { + EXPECT_THAT(NormalizeExponentInFloatString(""), Eq("")); + EXPECT_THAT(NormalizeExponentInFloatString("1e-12"), Eq("1e-12")); + EXPECT_THAT(NormalizeExponentInFloatString("1E+14"), Eq("1e+14")); + EXPECT_THAT(NormalizeExponentInFloatString("1e-0012"), Eq("1e-12")); + EXPECT_THAT(NormalizeExponentInFloatString("1.263E+014"), Eq("1.263e+14")); +} + +// The following two tests can't be DRY because they take different parameter +// types. +TEST_P(FloatProxyFloatTest, EncodeCorrectly) { + EXPECT_THAT( + NormalizeExponentInFloatString(EncodeViaFloatProxy(GetParam().first)), + Eq(GetParam().second)); +} + +TEST_P(FloatProxyDoubleTest, EncodeCorrectly) { + EXPECT_THAT( + NormalizeExponentInFloatString(EncodeViaFloatProxy(GetParam().first)), + Eq(GetParam().second)); +} + +INSTANTIATE_TEST_CASE_P( + Float32Tests, FloatProxyFloatTest, + ::testing::ValuesIn(std::vector, std::string>>({ + // Zero + {0.f, "0"}, + // Normal numbers + {1.f, "1"}, + {-0.25f, "-0.25"}, + {1000.0f, "1000"}, + + // Still normal numbers, but with large magnitude exponents. + {float(ldexp(1.f, 126)), "8.50706e+37"}, + {float(ldexp(-1.f, -126)), "-1.17549e-38"}, + + // denormalized values are printed as hex floats. + {float(ldexp(1.0f, -127)), "0x1p-127"}, + {float(ldexp(1.5f, -128)), "0x1.8p-128"}, + {float(ldexp(1.25, -129)), "0x1.4p-129"}, + {float(ldexp(1.125, -130)), "0x1.2p-130"}, + {float(ldexp(-1.0f, -127)), "-0x1p-127"}, + {float(ldexp(-1.0f, -128)), "-0x1p-128"}, + {float(ldexp(-1.0f, -129)), "-0x1p-129"}, + {float(ldexp(-1.5f, -130)), "-0x1.8p-130"}, + + // NaNs + {FloatProxy(uint32_t(0xFFC00000)), "-0x1.8p+128"}, + {FloatProxy(uint32_t(0xFF800100)), "-0x1.0002p+128"}, + + {std::numeric_limits::infinity(), "0x1p+128"}, + {-std::numeric_limits::infinity(), "-0x1p+128"}, + }))); + +INSTANTIATE_TEST_CASE_P( + Float64Tests, FloatProxyDoubleTest, + ::testing::ValuesIn( + std::vector, std::string>>({ + {0., "0"}, + {1., "1"}, + {-0.25, "-0.25"}, + {1000.0, "1000"}, + + // Large outside the range of normal floats + {ldexp(1.0, 128), "3.40282366920938e+38"}, + {ldexp(1.5, 129), "1.02084710076282e+39"}, + {ldexp(-1.0, 128), "-3.40282366920938e+38"}, + {ldexp(-1.5, 129), "-1.02084710076282e+39"}, + + // Small outside the range of normal floats + {ldexp(1.5, -129), "2.20405190779179e-39"}, + {ldexp(-1.5, -129), "-2.20405190779179e-39"}, + + // lowest non-denorm + {ldexp(1.0, -1022), "2.2250738585072e-308"}, + {ldexp(-1.0, -1022), "-2.2250738585072e-308"}, + + // Denormalized values + {ldexp(1.125, -1023), "0x1.2p-1023"}, + {ldexp(-1.375, -1024), "-0x1.6p-1024"}, + + // NaNs + {uint64_t(0x7FF8000000000000LL), "0x1.8p+1024"}, + {uint64_t(0xFFF0F00000000000LL), "-0x1.0fp+1024"}, + + // Infinity + {std::numeric_limits::infinity(), "0x1p+1024"}, + {-std::numeric_limits::infinity(), "-0x1p+1024"}, + + }))); + // TODO(awoloszyn): Add fp16 tests and HexFloatTraits. } diff --git a/test/TestFixture.h b/test/TestFixture.h index 5479532..40d5344 100644 --- a/test/TestFixture.h +++ b/test/TestFixture.h @@ -116,7 +116,7 @@ class TextToBinaryTestBase : public T { spvDiagnosticPrint(diagnostic); spvDiagnosticDestroy(diagnostic); } - EXPECT_EQ(SPV_SUCCESS, error); + EXPECT_EQ(SPV_SUCCESS, error) << text; const std::string decoded_string = decoded_text->str; spvTextDestroy(decoded_text); diff --git a/test/TextToBinary.Constant.cpp b/test/TextToBinary.Constant.cpp index 095c269..c14247e 100644 --- a/test/TextToBinary.Constant.cpp +++ b/test/TextToBinary.Constant.cpp @@ -32,8 +32,8 @@ #include #include -#include "gmock/gmock.h" #include "TestFixture.h" +#include "gmock/gmock.h" namespace { @@ -337,12 +337,12 @@ const int64_t kMaxSigned48Bit = (int64_t(1) << 47) - 1; const int64_t kMinSigned48Bit = -kMaxSigned48Bit - 1; TEST_P(RoundTripTest, Sample) { - EXPECT_THAT(EncodeAndDecodeSuccessfully(GetParam()), Eq(GetParam())); + EXPECT_THAT(EncodeAndDecodeSuccessfully(GetParam()), Eq(GetParam())) + << GetParam(); } -// TODO(dneto): Enable support once this works. INSTANTIATE_TEST_CASE_P( - DISABLED_OpConstantRoundTrip, RoundTripTest, + OpConstantRoundTrip, RoundTripTest, ::testing::ValuesIn(std::vector{ // 16 bit "%1 = OpTypeInt 16 0\n%2 = OpConstant %1 0\n", @@ -384,9 +384,8 @@ INSTANTIATE_TEST_CASE_P( "%1 = OpTypeFloat 64\n%2 = OpConstant %1 -1.79769e+308\n", })); -// TODO(dneto): Enable support once this works. INSTANTIATE_TEST_CASE_P( - DISABLED_OpSpecConstantRoundTrip, RoundTripTest, + OpSpecConstantRoundTrip, RoundTripTest, ::testing::ValuesIn(std::vector{ // 16 bit "%1 = OpTypeInt 16 0\n%2 = OpSpecConstant %1 0\n", diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp index 61020d2..c988a98 100644 --- a/test/TextToBinary.cpp +++ b/test/TextToBinary.cpp @@ -37,8 +37,10 @@ namespace { using libspirv::AssemblyContext; using libspirv::AssemblyGrammar; -using spvtest::TextToBinaryTest; using spvtest::AutoText; +using spvtest::Concatenate; +using spvtest::MakeInstruction; +using spvtest::TextToBinaryTest; using testing::Eq; TEST(GetWord, Simple) { @@ -431,13 +433,13 @@ TEST_F(TextToBinaryTest, WrongOpCode) { using TextToBinaryFloatValueTest = spvtest::TextToBinaryTestBase< ::testing::TestWithParam>>; -TEST_P(TextToBinaryFloatValueTest, NormalValues) { - const std::string assembly = "%1 = OpTypeFloat 32\n%2 = OpConstant %1 "; - const std::string input_string = assembly + GetParam().first; - const std::string expected_string = - assembly + std::to_string(GetParam().second) + "\n"; - const std::string decoded_string = EncodeAndDecodeSuccessfully(input_string); - EXPECT_EQ(expected_string, decoded_string); +TEST_P(TextToBinaryFloatValueTest, Samples) { + const std::string input = + "%1 = OpTypeFloat 32\n%2 = OpConstant %1 " + GetParam().first; + EXPECT_THAT(CompiledInstructions(input), + Eq(Concatenate({MakeInstruction(SpvOpTypeFloat, {1, 32}), + MakeInstruction(SpvOpConstant, + {1, 2, GetParam().second})}))); } INSTANTIATE_TEST_CASE_P( -- 2.7.4