Repository navigation
x86_64: branchless fxmin/fxmax via cmp + cmovcc - #1093
Open
ufo5260987423 wants to merge 1 commit into
Open
ufo5260987423 wants to merge 1 commit into
ufo5260987423 wants to merge 1 commit into
Conversation
mflatt
reviewed
Nov 5, 2026
| (constant-case ipmin-instruction | ||
| [(#t) | ||
| ; branchless cmp + cmovcc (see asm-ipminmax in x86_64.ss) | ||
| (bind #t (e1 e2) |
Contributor
There was a problem hiding this comment.
I think the #t here can be #f, since e1 and e2 are each referenced only once.
| [(e1 e2) | ||
| (constant-case ipmin-instruction | ||
| [(#t) | ||
| (bind #t (e1 e2) |
Add %ipmin/%ipmax value instructions selecting cmp/cmovcc sequences by dest/source aliasing; no scratch register is needed in any case. Gate behind ipmin-instruction (a6.def #t, default #f) and route the o=3 fxmin/fxmax inlines through it. cmov-op swaps the ax-ea-modrm-reg arguments relative to binary-op: unlike mov/add (dest = r/m), CMOVcc encodes the destination in the modrm reg field. Getting this wrong silently clobbers the source, and because the compiler's own frame-size bookkeeping uses fxmin/fxmax, the resulting heap corruption surfaces as inscrutable GC failures far from the cause.
ufo5260987423
force-pushed
the
pr-cmov-fxminmax
branch
from
November 6, 2026 00:31
8b39496 to
edde8f5
Compare
Contributor
Author
|
Thanks @mflatt - both suggestions applied: the cmov paths now use |
This branch has not been deployed
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.
This PR compiles 2-argument
fxmin/fxmaxat optimize level 3 on x86_64 to a compare and a conditional move instead of a compare and branch, eliminating branch-misprediction stalls when the comparison direction is data-dependent. Multi-argument calls inherit the same sequence through the existingreducepath.New
%ipmin/%ipmaxvalue instructions select acmp/cmovccsequence by destination/source aliasing:dest == src1:cmp dest, src2(flags = src2 − src1), thencmovl/cmovgfrom src2dest == src2:cmp src1, dest(flags = dest − src1), thencmovg/cmovlfrom src1 with the comparison direction swappeddest = src1first (aliasing nothing, so the move cannot clobber a source) and take the first caseNo scratch register is needed in any case. Unlike the flonum
flmin/flmaxstory in #1084, ties are unobservable here — fixnums are immediates, so either operand register gives the same result.cmovccis part of the original x86_64 baseline, so unlike the SSE4.1roundsdthere is no hardware-level portability question; the newipmin-instructionmachine constant (enabled ina6.def, defaulting to#findefault.def, likepopcount-instruction) keeps other architectures on the existing compare-and-branch inline.One encoding subtlety worth calling out for review:
cmov-opswaps theax-ea-modrm-regarguments relative tobinary-op— unlikemov/add(dest = r/m),CMOVccencodes the destination in the modrm reg field and the source as r/m, and the REX R/B bits follow the same swap. The comment incmov-opdocuments this, since getting it wrong silently clobbers the source.Performance (i7-8550U, optimize level 3, 2M operations per measurement, fixed seed with identical checksums both sides, gate flipped via
ipmin-instructionwith boot md5 verified per side):About 3x on unpredictable data; neutral on predictable data. The compiler itself uses
fxmin/fxmaxfor frame-size bookkeeping (small values, highly predictable), so per the const-rand row compile-time impact is approximately zero.No new mats:
fxmin/fxmaxare already covered bymats/fx.ms(boundary values, arity errors) andmats/cptypes.ms(inlining behavior), and the transformation preserves results exactly; both pass at all optimize levels. Happy to add targeted tests if preferred.A
release_notesentry is included.