vm: test int64 results against small-integer bounds first - #431
Open
shinyobjectz wants to merge 1 commit into
Open
shinyobjectz wants to merge 1 commit into
shinyobjectz wants to merge 1 commit into
Conversation
The integer fast paths of +, - and * checked every result against the int64 bounds, which are bignums on the BEAM. They now test the BEAM small-integer bounds (2^59 - 1, -2^59) first; a result inside them is already in int64 range, and anything else takes the unchanged full check. Adds int64_wrap_engines_test.exs: the register and constant forms at the int64 and small-integer bounds, with mixed int/float, under both engines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The dispatcher's integer fast paths for
+,-and*(@op_add/@op_subtract/@op_multiplyand their_kforms) check every result against@min_int/@max_intbefore writing it, so that they can wrap to int64. Both bounds are bignums on the BEAM, so every integer add pays a bignum comparison even when the result is small.Change
Numeric.to_signed_int64unchanged, so wraparound behaves exactly as before.Tests
test/lua/vm/int64_wrap_engines_test.exsruns under both engines. It passes operands as function parameters so that nothing is constant-folded. It coversa + b,a - banda * b, plusa + 1,a - 1anda * 2, which the peephole fuses to the_kopcodes.===.mainas well; they pin the behaviour this change must keep. As a check, raising the small bound above int64 made the compiled-engine cases fail.The full suite passes:
mix testgives 2875 passed, andmix test --include lua53 --include differential --include slowgives 2896 passed. One test,BootstrapTest"reset/0 ...", fails intermittently on this branch and onmainalike, in about one run in five.Benchmark
The machine is an Apple M4 with OTP 29 and Elixir 1.20.3,
MIX_ENV=prod. Each number is the median of 6 interleaved before/after rounds, in ms. Each round is itself the median of 15:timer.tcruns after 3 warmup runs. "budget" meansLua.new(max_instructions: 1_000_000_000).The top-level loop showed no measurable change.