Issue147 operator rshift

[almalkim]

Fixes operator>> and operator>>= for all big integer types: corrects result width, sign-bit detection, and stale upper bits

[almalkim]

Hi @AndrewGoodrich,
Can you please give this a look?
Thanks.

[AndrewGoodrich]

The code was written to optimize speed of calculations:

(1) Big integer values are stored in two's-complement form, including all the bits of the high order sc_digit, even past the width of the value.
(2) Where possible native types are used to calculate shifts, i.e., int for >= 32 bit values, int64 for >= 64 bit values, that is the reason for the if - else constructs. Note for sc_bigint and sc_biguint optimization actually removes the if or else statements just leaving the body of code within.
(3) Because values are stored in two's-complement form the high order digit always has proper sign-propagation, so casting to an int eliminates the need to shift the high order digit based on the width of the value.
(4) Reducing the width of results where possible reduces the number of digits that must be processed in subsequent calculations.

[almalkim]

Hi @AndrewGoodrich,

Thank you for the detailed explanation. I've revised the PR based on your feedback.

Regarding the sign detection, you are correct that this works reliably as long as there are no stale bits in the high order digit. I have ensured that is the case through the fixes in the companion PR: [https://github.com//pull/148]

As for the result width narrowing in [operator>>] I understand this is an intentional optimization for performance. However, it does not conform to IEEE 1666-2023 S 7.2.8, which specifies that the result width shall equal the left operand width. If strict conformance to the standard is not a requirement, I'm happy to close this PR.

Thanks.

[almalkim]

Hi @AndrewGoodrich,
anything else needs to be fixed for this?

[AndrewGoodrich]

I have submitted a pull request to address the IEEE 1666 width compliance, that includes the optimizations for width data types. That is the last change I can think of.

[AndrewGoodrich]

The proposed changes for the library components have been implemented in the private repository, but are different in that they still use conditional logic based on the value of W. The results are the same and maintain the proper width, but will execute faster.

The new test needs to be added to the test suite. I will do that in the private repository and then we can merge the private one into the public one. At that point this pull request can be removed. Thanks for the work on this.