1

I'm making a class I think I have it mostly working correctly, but the division has stumped me. I have the part that I'm working on in the main I got it off of here. Unfortunately, I can't get the division code to work with my class. It works fine with uin64 and I don't know what part of my class isn't working.

I have found many bugs in my code and fixed them. I also did a thorough test of the code that the division function relies on but it still refuses to work

class uint128_t {
public:
    unsigned long long int data[2];
    uint128_t() {
        data[1] = 0;
        data[0] = 0;
    }
    uint128_t operator~() {
        uint128_t out;
        out.data[0] = ~data[0];
        out.data[1] = ~data[1];
        return out;
    }
        uint128_t operator+(const uint128_t& b) {
        uint128_t out;
        out.data[1] = data[1] + b.data[1];
        out.data[0] = data[0] + b.data[0];
        if (b.data[0] > (UINT8_MAX - data[0]))out.data[1]++;
        return out;
    }
    uint128_t operator+(const unsigned long long int& b) {
        uint128_t out;
        out.data[1] = data[1];
        out.data[0] = data[0] + b;
        if (b > (UINT64_MAX - data[0]))out.data[1]++;
        return out;
    }
    uint128_t operator-(const uint128_t& b) {
        uint128_t out;
        out.data[1] = data[1] + ~b.data[1];
        out.data[0] = data[0] + ~b.data[0];
        if (b.data[0] > (UINT8_MAX - data[0]))out.data[1]++;
        out.data[0]++;
        out.data[1]++;
        return out;
    }
    bool operator<(const uint128_t& b) {
        if (data[1] == b.data[1]) {
            return (data[0] < b.data[0]) ? true : false;
        }
        else return (data[1] < b.data[1]) ? true : false;
    }   
        bool operator>=(const uint128_t& b) {
        if (data[1] == b.data[1]) {
            return (data[0] >= b.data[0]) ? true : false;
        }
        else return (data[1] > b.data[1]) ? true : false;
    }
    uint128_t &operator=(const int& b) {
        data[1] = 0;
        data[0] = b;
        return *this;
    }
    uint128_t& operator=(const uint128_t& b) {
        data[1] = b.data[1];
        data[0] = b.data[0];
        return *this;
    }
};

int main()
{

    uint128_t divisor;
    divisor = 2;
    uint128_t quot, rem, t;
    int bits_left = 128;

    quot = 128;
    rem = 0;
    do {
        // (rem:quot) << 1
        t = quot;
        quot = quot + quot;
        rem = rem + rem + (quot < t);

        if (rem >= divisor) {
            rem = rem - divisor;
            quot = quot + 1;
        }
        bits_left--;
    } while (bits_left);
    std::cout << std::bitset<64>(quot.data[1]) << std::bitset<64>(quot.data[0]) << std::endl;
}

So 128(quot)/2(divisor) = 64 but every time I run it I get some gibberish. But if I change the type from my class to uint64 it works fine

phuclv
  • 27,258
  • 11
  • 104
  • 360
  • You say that the division function doesn't work right, but what *does* it do? Does it give the wrong answer? If so, can you find any pattern in what wrong answer it gives? Does it crash the program, and if so what error message if any are you getting and what does a debugger say? Does it fail to compile, and if so, what's the error message? – Daniel H Apr 15 '19 at 03:23
  • 1
    carries should be easer to get than that. Just use `out.data[0] = data[0] + b.data[0]; out.data[1] = data[1] + b.data[1] + out.data[0] < data[0];` [An efficient way to do basic 128 bit integer calculations in C++?](https://stackoverflow.com/q/27261291/995714), [Access the flags without inline assembly?](https://stackoverflow.com/q/26807185/995714) – phuclv Apr 15 '19 at 03:31
  • @phuclv If you add a third type, and you get a carry and you got a carry before it breaks. I really like it though and I completely didn't think of that. example of what im talking about ```uint8_t a[5], b[5], c[5]; memset(a, 0, 5); memset(b, 0, 5); memset(c, 0, 5); ((uint16_t)&a) = UINT16_MAX; ((uint16_t)&b) = UINT16_MAX; c[0] = b[0] + a[0]; for (size_t i = 1;(c[i - 1] < b[i - 1]); i++) { c[i] = a[i] + b[i] + (c[i - 1] < b[i - 1]); }std::cout << std::bitset<24>(((uint32_t)&c)) << std::endl << " ^should be 1 here";``` – Jesse Taube Apr 19 '19 at 02:57

1 Answers1

2

You have several places where UINT8_MAX is used when UINT64_MAX should appear instead. The operator+ is one of those cases.

uint128_t operator+(const uint128_t& b) {
    uint128_t out;
    out.data[1] = data[1] + b.data[1];
    out.data[0] = data[0] + b.data[0];
    if (b.data[0] > (UINT8_MAX - data[0]))out.data[1]++;
    //               ^^^^^^^ here
    // should be UNIT64_MAX
    return out;
}

And

uint128_t operator-(const uint128_t& b) {
    uint128_t out;
    out.data[1] = data[1] + ~b.data[1];
    out.data[0] = data[0] + ~b.data[0];
    if (b.data[0] > (UINT8_MAX - data[0]))out.data[1]++;
    //               ^^^^^^^ here
    // should be UNIT64_MAX
    out.data[0]++;
    out.data[1]++;
    return out;
}

Writing big-int, or int128, can be a good learning experience, but you should be aware of existing libraries that do the same, such as boost int128_t.

Also, write a set of simple unit test. Each unit test will check at most one operator at a time. Check interesting cases like adding values that almost need carry, those that barely need carry, and so on. Such tests would have found the bugs much easier.

Michael Veksler
  • 7,337
  • 1
  • 16
  • 29