On Thu, 12 Sep 2024 11:23:24 GMT, fabioromano1 <d...@openjdk.org> wrote:
>> src/java.base/share/classes/java/math/MutableBigInteger.java line 722:
>>
>>> 720: * {@code (resPos <= offset || resPos >= offset + intLen)}.
>>> 721: */
>>> 722: private final void primitiveRightShift(int n, int[] result, int
>>> resPos) {
>>
>> Suggestion:
>>
>> private final void primitiveRightShift(int n, int[] result, int resOff) {
>>
>> or something more similar to "offset".
>>
>> Same for the left shift.
>
> What about `resFrom`, like the old `MBI.copyAndshift()` method?
Up to you, the current `resPos` is OK as well.
>> src/java.base/share/classes/java/math/MutableBigInteger.java line 753:
>>
>>> 751: * {@code (resPos <= offset || resPos >= offset + intLen)}.
>>> 752: */
>>> 753: private final void primitiveLeftShift(int n, int[] result, int
>>> resPos) {
>>
>> I think this method can be made more symmetrical w.r.t.
>> `primitiveRightShift()` if starting from the right (least significant `int`).
>
> Yes, it could, but the problem is that in this way the precondition `(resPos
> <= offset || resPos >= offset + intLen)` would be no longer correct, and in
> particular `resPos <= offset` is used by `MBI.leftShift()` if `result ==
> value`.
I mean something like
private void primitiveRightShift(int n, int[] result, int resPos) {
int[] val = value;
int n2 = 32 - n;
int c = 0;
for (int i = 0; i < intLen; i++) {
int b = val[offset + i];
result[resPos + i] = c << n2 | b >>> n;
c = b;
}
}
and
private void primitiveLeftShift(int n, int[] result, int resPos) {
int[] val = value;
int n2 = 32 - n;
int c = 0;
for (int i = intLen - 1; i >= 0; i--) {
int b = val[offset + i];
result[resPos + i] = c >>> n2 | b << n;
c = b;
}
They are compatible with the precondition, right?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756722886
PR Review Comment: https://git.openjdk.org/jdk/pull/20008#discussion_r1756721676
