SVInt operator< fix.

[Krym4s]

Hello, SVInt compares incorrectly with minimum signed int value. These test cases are failed on current main branch

CHECK(SVInt(32, -2147483648, 1) < SVInt(32, -1, 1));
CHECK(SVInt(32, -1, 1) > SVInt(32, -2147483648, 1));

It is because of negation of minimum signed int value is impossible (INT_MIN = -INT_MAX-1). This fact is used here. It is better to compare bit inversions of negative values as

-var = ~var + 1

so every value can be represented.

This patch also includes function for representing minimum and maximum integer values.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.00%. Comparing base (3c5e5e0) to head (d0c091f).

Additional details and impacted files

[](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) ```diff @@ Coverage Diff @@ ## master #928 +/- ## ========================================== - Coverage 94.00% 94.00% -0.01% ========================================== Files 189 189 Lines 48428 48423 -5 ========================================== - Hits 45527 45522 -5 Misses 2901 2901 ``` | [Files](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) | Coverage Δ | | |---|---|---| | [source/numeric/SVInt.cpp](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?src=pr&el=tree&filepath=source%2Fnumeric%2FSVInt.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-c291cmNlL251bWVyaWMvU1ZJbnQuY3Bw) | `98.78% <100.00%> (-0.01%)` | :arrow_down: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). Last update [3c5e5e0...d0c091f](https://app.codecov.io/gh/MikePopoloski/slang/pull/928?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski).

[Krym4s]

Added new commit with shared library build fix. Also added some tests.

[MikePopoloski]

Thanks for the PR; I definitely should have had test coverage of this case. I think the extra codepath here is unnecessary though; if both sides are negative then we can fall through to the normal unsigned handling case, since negative numbers will compare correctly when both are negative and treated as unsigned. In fact I can probably clean up these comparison functions a bit more to make them more efficient anyway.

I'm not entirely convinced of the need for the getMinValue / getMaxValue functions added here. They seem unrelated to the bug? At the very least the overloads that accept another SVInt are confusing, since the value of that parameter doesn't contribute to the result.

[Krym4s]

Thank you for your answer. Yes, getMinValue and getMaxValue are not related to this bug, so it is better to remove it from this PR, but I think that this methods are useful, so maybe it is possible to add them later.

In new commit I handled case when only one of two signed numbers are negative. Also I added tests for this case. In case two values are negative we fall through to the normal unsigned handling case.

[MikePopoloski]

Thanks, lgtm