Skip to content

UInt256 long digits record#9677

Merged
lu-pinto merged 36 commits intobesu-eth:mainfrom
thomas-quadratic:feat/uint256-as-record
Mar 1, 2026
Merged

UInt256 long digits record#9677
lu-pinto merged 36 commits intobesu-eth:mainfrom
thomas-quadratic:feat/uint256-as-record

Conversation

@thomas-quadratic
Copy link
Copy Markdown
Contributor

@thomas-quadratic thomas-quadratic commented Jan 23, 2026

PR description

This PR cumulates a few improvements/refactoring for UInt256 modular arithmetics:

  • digits (limbs) are big-endian ordered.
  • digits use longs rather than ints as primitive type.
  • UInt256 is a record storing its digits as fields rather than in an array.
  • division algorithm follows methods from the GMP division paper since widening from long is not possible.
  • fromBytesBE is improved to give better performance on worst cases.

Long limbs are more efficient because they divide by 2 the number of digits, thus reducing the number of steps in arithmetics operations. However, it complexifies the implementation for division as widening is not possible anymore. Fortunately, quotient estimates algorithms that avoids widening exists, see division paper.

Records are chosen to represent fixed-width UInt256 because of potential future support for Vahalla value records, which would require almost no change to the code.

Here are the benchmarks compared to the actual implementation:

Operation Case current (ns/op) new (ns/op) gain (%)
Mod FULL_RANDOM 106.042 84.005 21%
Mod WORST 149.179 95.7 36%

Current status

Currently working and tested ops:

  • AddMod
  • Mod
  • MulMod
  • SMod

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests
  • hive tests: Engine or other RPCs modified?

@thomas-quadratic thomas-quadratic changed the title Feat/uint256 as record UInt256 long digits record Jan 23, 2026
@thomas-quadratic
Copy link
Copy Markdown
Contributor Author

Updated benchmarks with the new commits that support the other operations: AddMod/MulMod/SMod

Operation Case current (ns/op) new (ns/op) gain (%)
Mod FULL_RANDOM 106.042 84.005 21%
Mod WORST 149.179 95.7 36%
SMod FULL_RANDOM 141.778 101.1 29%
SMod WORST 170.822 101.1 41%
AddMod FULL_RANDOM 179.559 137.073 24%
AddMod WORST 187.805 137.073 27%
MulMod FULL_RANDOM 259.265 186.501 29%
MulMod WORST 344.663 222.37 41%

@thomas-quadratic
Copy link
Copy Markdown
Contributor Author

Changing single digit quotient estimates using 3 digits by 2 digits (div3by2) to 2 digits by 1 digit (div2by1).
Improved benchmarks across the board:

Operation Case current (ns/op) new (ns/op) gain (%)
Mod FULL_RANDOM 106.042 81.032 24%
Mod WORST 149.179 86.942 42%
SMod FULL_RANDOM 141.778 97.15 31%
SMod WORST 170.822 97.15 43%
AddMod FULL_RANDOM 179.559 130.147 28%
AddMod WORST 187.805 130.147 31%
MulMod FULL_RANDOM 259.265 177.442 32%
MulMod WORST 344.663 209.105 39%

@thomas-quadratic thomas-quadratic marked this pull request as ready for review January 30, 2026 07:44
@macfarla macfarla moved this to In Progress in Performance Feb 1, 2026
@siladu
Copy link
Copy Markdown
Contributor

siladu commented Feb 12, 2026

Code review

Found 1 issue:

  1. UInt512.isUInt64() uses bitwise AND (&) instead of OR (|) to check if all high limbs are zero. With AND, the expression (u7 & u6 & u5 & u4 & u3 & u2 & u1) == 0 can return true even when multiple high limbs are non-zero, as long as they don't share a common bit (e.g., u7=1, u6=2 gives 1 & 2 = 0). This causes incorrect results in Modulus64 arithmetic shortcuts (addMod, mod, mulMod) that rely on this check. The correct operator is |, consistent with UInt256.isUInt64() at line 295 which already uses (u1 | u2 | u3) == 0.

https://github.com/hyperledger/besu/blob/d86a15e7a7a9e3441d2d85ef4f96a4dc58413519/evm/src/main/java/org/hyperledger/besu/evm/UInt256.java#L941-L944

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@siladu
Copy link
Copy Markdown
Contributor

siladu commented Feb 12, 2026

Code review

Found 1 issue:

  1. UInt512.isUInt64() uses bitwise AND (&) instead of OR (|) to check if all high limbs are zero. With AND, the expression (u7 & u6 & u5 & u4 & u3 & u2 & u1) == 0 can return true even when multiple high limbs are non-zero, as long as they don't share a common bit (e.g., u7=1, u6=2 gives 1 & 2 = 0). This causes incorrect results in Modulus64 arithmetic shortcuts (addMod, mod, mulMod) that rely on this check. The correct operator is |, consistent with UInt256.isUInt64() at line 295 which already uses (u1 | u2 | u3) == 0.

Unit test showing bug here https://github.com/thomas-quadratic/besu/pull/5/changes

long x63 = (x + 1) >>> 1;
long v0 = LUT[x9 - 256] & 0xFFFFL;
long v1 = (v0 << 11) - ((v0 * v0 * x40) >>> 40) - 1;
long v2 = (v1 << 13) + ((v1 * ((1L << 60) - v1 * x40)) >>> 47);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for doing computation of a constant number here:

Suggested change
long v2 = (v1 << 13) + ((v1 * ((1L << 60) - v1 * x40)) >>> 47);
long v2 = (v1 << 13) + ((v1 * (0x1000000000000000L- v1 * x40)) >>> 47);

I you want maybe just a static constant just above reciprocal method would do as well

Copilot AI review requested due to automatic review settings February 24, 2026 14:27
Before, limbs were stored in little-endian.
But to use Arrays.mismatch to our advantage, it is better to have it big-endian.
This commit makes UInt256.java big-endian in limbs.
We still need to migrate all tests and benchmark.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Also added tests that were failing and now pass.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Small cleaning up of the private methods for addition and compareLimbs.
Should be easier for the compiler.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
UInt256 used int[] for limbs, primarily for simplicity, e.g. having the possibility to widen to long.
However, methods exists to work with long[] and no widening. This commit implements long limbs.

To avoid widening, we do:

  1. add: overflow check
  2. mul: native multiplyHigh (compiled to assembly mulq)
  3. div: more complicated, see the gnump division paper.

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
thomas-quadratic and others added 18 commits February 24, 2026 14:35
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Add refactor-focused jqwik properties for the UInt256 4xlong record
representation.

Run:
  ./gradlew :evm:test --tests org.hyperledger.besu.evm.UInt256RecordProp

Observed failures (at introduction time):
- property_shiftLeft_matches_big_integer_mod_2_256 (seed 1123581321)
  Shrunk: a = 0x00000000000000000000000000000000000000000000000000000000000000ff
          shift = -512
  Expected: 0x00..00
  Got:      0xff..ff

- property_shiftRight_matches_big_integer_mod_2_256 (seed 867530900)
  Shrunk: a = 0x00000000000000000000000000000000000000000000000000000000000000ff
          shift = -512
  Expected: 0x00..00
  Got:      0x00..ff

- property_mod_matches_big_integer_unsigned (seed 123456789)
  Sample:  arg0 = byte[] [-128, 0, 0, 0, 0, 0, 0, 0, -128]
          arg1 = 64-byte MSB-heavy pattern (truncated to 32)
  Expected: 0x00..00
  Got:      0x00..80

- property_signedMod_matches_evm_semantics (seed 987654321)
  Sample:  arg0 = byte[] [-128, 0, 0, 0, 0, 0, 0, 0, -128]
          arg1 = byte[] [-128]
  Expected: 0x00..00
  Got:      0x00..80

- property_addMod_matches_big_integer_unsigned (seed 42424242)
  Sample:  arg0 = byte[] []
          arg1 = byte[] [-128, 0, 0, 0, 0, 0, 0, 0, -128]
          arg2 = 64-byte MSB-heavy pattern (truncated to 32)
  Expected: 0x00..00
  Got:      0x00..80

Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
UInt256.shiftLeft/shiftRight are only defined for 0 <= shift < 64.
Gate the corresponding properties with an assumption so they don't
assert EVM-wide shift semantics.

This removes shift-related false positives; remaining failures are
isolated to mod/signedMod/addMod.

Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@lu-pinto lu-pinto force-pushed the feat/uint256-as-record branch from 4ac107c to c0dc03f Compare February 24, 2026 14:35
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
@lu-pinto
Copy link
Copy Markdown
Contributor

lu-pinto commented Feb 24, 2026

I'm running a hoodi sync just as a smoke test. will merge when that succeeds

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lu-pinto lu-pinto enabled auto-merge (squash) March 1, 2026 10:35
@lu-pinto lu-pinto merged commit 57a7e7d into besu-eth:main Mar 1, 2026
46 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Performance Mar 1, 2026
@lu-pinto lu-pinto mentioned this pull request Mar 2, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants