Skip to content

Commit

Permalink
fmt: fuzz, remove some unnecessary busywork
Browse files Browse the repository at this point in the history
Removing some signed checks and logic where we've already guaranteed the
values to be positive.  Indeed, in these cases, if a negative value got
through (per my realisation in the signed fuzz harness), it would cause
an infinite loop due to flooring division.
  • Loading branch information
charlottia authored and mwkmwkmwk committed Aug 11, 2023
1 parent 2ae551c commit 3571bf2
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 20 deletions.
5 changes: 1 addition & 4 deletions backends/cxxrtl/cxxrtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -905,10 +905,7 @@ std::ostream &operator<<(std::ostream &os, const value_formatted<Bits> &vf)
buf += '0';
while (!val.is_zero()) {
value<Bits> quotient;
if (negative)
val.signedDivideWithRemainder(value<Bits>{10u}, quotient);
else
val.divideWithRemainder(value<Bits>{10u}, quotient);
val.divideWithRemainder(value<Bits>{10u}, quotient);
buf += '0' + val.template slice<3, 0>().val().template get<uint8_t>();
val = quotient;
}
Expand Down
4 changes: 2 additions & 2 deletions kernel/fmt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -680,8 +680,8 @@ std::string Fmt::render() const
if (absvalue.is_fully_zero())
buf += '0';
while (!absvalue.is_fully_zero()) {
buf += '0' + RTLIL::const_mod(absvalue, 10, part.signed_, false, 4).as_int();
absvalue = RTLIL::const_div(absvalue, 10, part.signed_, false, absvalue.size());
buf += '0' + RTLIL::const_mod(absvalue, 10, false, false, 4).as_int();
absvalue = RTLIL::const_div(absvalue, 10, false, false, absvalue.size());
}
if (negative || part.plus)
buf += negative ? '-' : '+';
Expand Down
1 change: 1 addition & 0 deletions tests/fmt/fuzz/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ add_executable(
x_test
x_test.cc
../../../libs/bigint/BigUnsigned.cc
../../../libs/bigint/BigInteger.cc
)

link_fuzztest(x_test)
Expand Down
65 changes: 51 additions & 14 deletions tests/fmt/fuzz/x_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,33 @@
#include <sstream>
#include "backends/cxxrtl/cxxrtl.h"
#include "libs/bigint/BigUnsigned.hh"
#include "libs/bigint/BigInteger.hh"

using namespace cxxrtl_yosys;

void Formats128BitUnsignedIntegers(uint64_t x, uint64_t y) {
// Compare output to actual BigUnsigned.
value<64> vs;
void Formats128BitUnsignedIntegers(chunk_t x0, chunk_t x1, chunk_t x2, chunk_t x3)
{
// Compare output to BigUnsigned.
value<128> v;
vs.set(x);
v = v.blit<127, 64>(vs);
vs.set(y);
v = v.blit<63, 0>(vs);
v = v.blit<127, 64>(value<64>{x1, x0});
v = v.blit<63, 0>(value<64>{x3, x2});

std::ostringstream oss;
oss << value_formatted<128>(v, false, false, ' ', 0, 10, false, false, false);
auto actual = oss.str();

BigUnsigned b(x);
b.bitShiftLeft(b, 64);
b.bitOr(b, y);
BigUnsigned u;
u.bitShiftLeft(v.slice<127, 64>().val().get<uint64_t>(), 64);
u.bitOr(u, v.slice<63, 0>().val().get<uint64_t>());

std::string expected;

if (b.isZero()) {
if (u.isZero()) {
expected = "0";
} else {
while (!b.isZero()) {
expected += '0' + (b % 10).toInt();
b /= 10;
while (!u.isZero()) {
expected += '0' + (u % 10).toInt();
u /= 10;
}
std::reverse(expected.begin(), expected.end());
}
Expand All @@ -40,3 +39,41 @@ void Formats128BitUnsignedIntegers(uint64_t x, uint64_t y) {
}
FUZZ_TEST(CxxrtlDivisionFuzz, Formats128BitUnsignedIntegers);


void Formats128BitSignedIntegers(chunk_t x0, chunk_t x1, chunk_t x2, chunk_t x3) {
// Compare output to BigInteger.
value<128> v;
v = v.blit<127, 64>(value<64>{x1, x0});
v = v.blit<63, 0>(value<64>{x3, x2});

std::ostringstream oss;
oss << value_formatted<128>(v, false, false, ' ', 0, 10, true, false, false);
auto actual = oss.str();

BigUnsigned u;
bool negative = v.is_neg();
if (negative)
v = v.neg();
u.bitShiftLeft(v.slice<127, 64>().val().get<uint64_t>(), 64);
u.bitOr(u, v.slice<63, 0>().val().get<uint64_t>());

std::string expected;
if (u.isZero()) {
expected = "0";
} else {
// Note that we never actually do division of negative numbers: our division
// routines are flooring, not truncating, so dividing by 10 repeatedly won't
// necessarily ever get to zero.
while (!u.isZero()) {
expected += '0' + (u % 10).toInt();
u /= 10;
}
if (negative) {
expected += '-';
}
std::reverse(expected.begin(), expected.end());
}

EXPECT_EQ(actual, expected);
}
FUZZ_TEST(CxxrtlDivisionFuzz, Formats128BitSignedIntegers);

0 comments on commit 3571bf2

Please sign in to comment.