[arm64|turbofan|maglev] Implement fast path for modulo [v8/v8 : main]
0 views
Skip to first unread message
Marja Hölttä (Gerrit)
Feb 23, 2026, 6:24:12 AM (yesterday) Feb 23
to Victor Gomes, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Victor Gomes
Marja Hölttä voted and added 1 comment![]( "Open in Gerrit")
Votes added by Marja Hölttä
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Victor Gomes
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Gomes (Gerrit)
Feb 23, 2026, 8:36:04 AM (yesterday) Feb 23
to Marja Hölttä, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Marja Hölttä
Victor Gomes voted and added 6 comments![]( "Open in Gerrit")
Votes added by Victor Gomes
| Code-Review | +1 |
6 comments
Patchset-level comments
Victor Gomes . resolved
LGTM
File src/compiler/backend/arm64/code-generator-arm64.cc
Line 2246, Patchset 5 (Latest):
// Check if right is a positive integer.
__ Fcvtzu(x_scratch, right);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(right, d_temp);
__ B(ne, &slow);
__ Fcmp(right, 0.0);
__ B(le, &slow);Victor Gomes . unresolved
I bet most of the time we do module a fixed integer, so we could even avoid this!
Line 2262, Patchset 5 (Latest):
__ bind(&slow);Victor Gomes . unresolved
Do we need AllowExternalCallThatCantCauseGC here as well?
File src/maglev/arm64/maglev-ir-arm64.cc
Line 799, Patchset 5 (Latest):
// Check if right is a positive integer.
__ Fcvtzu(x_scratch, right);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(right, d_temp);
__ B(ne, &slow);
__ Fcmp(right, 0.0);
__ B(le, &slow);
Victor Gomes . unresolved
For Maglev it should be easy to check if right is a constant.
Line 808, Patchset 5 (Latest):
__ Fdiv(d_temp, left, right);
__ Frintz(d_temp, d_temp);
__ Fmsub(out, d_temp, right, left);Victor Gomes . unresolved
I guess if we were able to prove (statically) that both are integers, we would have emitted Int32ModulusWithOverflow instead?
Line 785, Patchset 5 (Latest):
Label slow, done_mod;
{
UseScratchRegisterScope temps(masm);
DoubleRegister d_temp = temps.AcquireD();
Register x_scratch = temps.AcquireX();
// Check if left is a positive integer.
__ Fcvtzu(x_scratch, left);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(left, d_temp);
__ B(ne, &slow);
__ Fcmp(left, 0.0);
__ B(le, &slow);
// Check if right is a positive integer.
__ Fcvtzu(x_scratch, right);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(right, d_temp);
__ B(ne, &slow);
__ Fcmp(right, 0.0);
__ B(le, &slow);
// Both are positive integers.
__ Fdiv(d_temp, left, right);
__ Frintz(d_temp, d_temp);
__ Fmsub(out, d_temp, right, left);
__ B(&done_mod);
}
__ bind(&slow);
{
AllowExternalCallThatCantCauseGC scope(masm);
__ CallCFunction(ExternalReference::mod_two_doubles_operation(), 0, 2);
}
__ bind(&done_mod);Victor Gomes . unresolved
Since this is common for both maglev and code-generator, should we add it to the MacroAssembler instead?
Related details
Attention is currently required from:
- Marja Hölttä
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Gomes (Gerrit)
4:46 AM (4 hours ago) 4:46 AM
to Marja Hölttä, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Marja Hölttä
Victor Gomes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Victor Gomes
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Marja Hölttä
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marja Hölttä (Gerrit)
4:50 AM (4 hours ago) 4:50 AM
to Victor Gomes, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Marja Hölttä added 5 comments![]( "Open in Gerrit")
File src/compiler/backend/arm64/code-generator-arm64.cc
// Check if right is a positive integer.
__ Fcvtzu(x_scratch, right);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(right, d_temp);
__ B(ne, &slow);
__ Fcmp(right, 0.0);
__ B(le, &slow);Victor Gomes . resolved
I bet most of the time we do module a fixed integer, so we could even avoid this!
Marja Hölttä
I'll leave that as a follow up. In lazy-collections, both left and right are not constants (we compute the Erastothenes sieve like 123 % 2, 123 % 3 etc.), so it won't help there. I'm adding a TODO.
Do we need AllowExternalCallThatCantCauseGC here as well?
Marja Hölttä
Using the FrameScope as discussed
File src/maglev/arm64/maglev-ir-arm64.cc
Line 799, Patchset 5:
// Check if right is a positive integer.
__ Fcvtzu(x_scratch, right);
__ Scvtf(d_temp, x_scratch);
__ Fcmp(right, d_temp);
__ B(ne, &slow);
__ Fcmp(right, 0.0);
__ B(le, &slow);
Victor Gomes . resolved
For Maglev it should be easy to check if right is a constant.
Marja Hölttä
Ditto
Line 808, Patchset 5:
__ Fdiv(d_temp, left, right);
__ Frintz(d_temp, d_temp);
__ Fmsub(out, d_temp, right, left);Victor Gomes . resolved
I guess if we were able to prove (statically) that both are integers, we would have emitted Int32ModulusWithOverflow instead?
Marja Hölttä
will look into this when I look into the follow ups...
Line 785, Patchset 5:
Label slow, done_mod;Victor Gomes . resolved
Since this is common for both maglev and code-generator, should we add it to the MacroAssembler instead?
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
chromeperf@appspot.gserviceaccount.com (Gerrit)
5:49 AM (3 hours ago) 5:49 AM
to Marja Hölttä, Victor Gomes, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Attention needed from Marja Hölttä
Message from chrom...@appspot.gserviceaccount.com![]( "Open in Gerrit")
📍 Job mac-m1_mini_2020-perf/jetstream-main.crossbench complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/15ef8395f10000
Related details
Attention is currently required from:
- Marja Hölttä
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Marja Hölttä (Gerrit)
6:37 AM (2 hours ago) 6:37 AM
to Victor Gomes, chrom...@appspot.gserviceaccount.com, V8 LUCI CQ, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
Marja Hölttä voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
V8 LUCI CQ (Gerrit)
6:40 AM (2 hours ago) 6:40 AM
to Marja Hölttä, Victor Gomes, chrom...@appspot.gserviceaccount.com, dmercadi...@chromium.org, leszek...@chromium.org, v8-re...@googlegroups.com, verwaes...@chromium.org, victorgo...@chromium.org
V8 LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[arm64|turbofan|maglev] Implement fast path for modulo
Note the existing test case div-mod.js.
Change-Id: I663f2868a94cb750abecf342e77d69011518fccb
Reviewed-by: Victor Gomes <victo...@chromium.org>
Commit-Queue: Marja Hölttä <ma...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#105410}
Files:
- M src/codegen/arm64/macro-assembler-arm64.cc
- M src/codegen/arm64/macro-assembler-arm64.h
- M src/compiler/backend/arm64/code-generator-arm64.cc
- M src/maglev/arm64/maglev-ir-arm64.cc
Change size: M
Delta: 4 files changed, 53 insertions(+), 8 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Victor Gomes
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages