Skip to content

cmd/compile: zero the SETcc destination ahead of the compare on amd64 - #80574

Open
gaul wants to merge 1 commit into
golang:masterfrom
gaul:setcc-zext
Open

cmd/compile: zero the SETcc destination ahead of the compare on amd64#80574
gaul wants to merge 1 commit into
golang:masterfrom
gaul:setcc-zext

Conversation

@gaul

@gaul gaul commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Materializing a bool into a whole register currently widens after the
fact:

    CMPQ    BX, AX
    SETLS   DL
    MOVBLZX DL, DX

SETcc writes only DL, so DX's upper bits have to be cleared afterwards.
Intel's optimization manual recommends zeroing the register ahead of the
flag-setting instruction instead, which drops the MOVBLZX entirely:

    XORL    DX, DX
    CMPQ    BX, AX
    SETLS   DL

The rewrite is a statement about a machine register rather than about a
value -- it is legal exactly when the register SETcc writes is dead
across the comparison being hoisted over -- so it runs as a new pass
after regalloc. Expressing it earlier would make the zero a value
needing a register of its own, trading an instruction for register
pressure in the code that has least to spare.

Two shapes are declined. When the flag setter reads the SETcc's own
destination -- regalloc often reuses a dying operand for the bool --
there is nowhere to put the zeroing. When the flag setter consumes flags
itself, as an ADCQ/SBBQ link of a carry chain does, flags are still live
in front of it and the zero would have to be materialized as a 5-byte
MOVL $0 that saves neither space nor a uop.

Besides being one instruction and at least one byte shorter, the new
form breaks the false dependency that SETcc's partial-register write
carries on the destination's previous value. In the ML-DSA NTT butterfly
that dependency was loop-carried through the Montgomery conditional
subtract's IMUL, so removing it is worth considerably more than the
instruction itself:

    Sign/ML-DSA-44     246126ns -> 221760ns   -9.9%
    Keygen/ML-DSA-44   105230ns ->  97267ns   -7.6%

(medians of 6 interleaved runs each, AMD Ryzen AI MAX+ 395)

cmd/go's .text shrinks by 192 bytes and no function in cmd/go or gofmt
grows. x86lint's "suboptimal SETcc zero-extension" findings on cmd/go
fall from 384 to 235, the remainder being the two declined shapes.

Materializing a bool into a whole register currently widens after the
fact:

	CMPQ    BX, AX
	SETLS   DL
	MOVBLZX DL, DX

SETcc writes only DL, so DX's upper bits have to be cleared afterwards.
Intel's optimization manual recommends zeroing the register ahead of the
flag-setting instruction instead, which drops the MOVBLZX entirely:

	XORL    DX, DX
	CMPQ    BX, AX
	SETLS   DL

The rewrite is a statement about a machine register rather than about a
value -- it is legal exactly when the register SETcc writes is dead
across the comparison being hoisted over -- so it runs as a new pass
after regalloc. Expressing it earlier would make the zero a value
needing a register of its own, trading an instruction for register
pressure in the code that has least to spare.

Two shapes are declined. When the flag setter reads the SETcc's own
destination -- regalloc often reuses a dying operand for the bool --
there is nowhere to put the zeroing. When the flag setter consumes flags
itself, as an ADCQ/SBBQ link of a carry chain does, flags are still live
in front of it and the zero would have to be materialized as a 5-byte
MOVL $0 that saves neither space nor a uop.

Besides being one instruction and at least one byte shorter, the new
form breaks the false dependency that SETcc's partial-register write
carries on the destination's previous value. In the ML-DSA NTT butterfly
that dependency was loop-carried through the Montgomery conditional
subtract's IMUL, so removing it is worth considerably more than the
instruction itself:

	Sign/ML-DSA-44     246126ns -> 221760ns   -9.9%
	Keygen/ML-DSA-44   105230ns ->  97267ns   -7.6%

(medians of 6 interleaved runs each, AMD Ryzen AI MAX+ 395)

cmd/go's .text shrinks by 192 bytes and no function in cmd/go or gofmt
grows. x86lint's "suboptimal SETcc zero-extension" findings on cmd/go
fall from 384 to 235, the remainder being the two declined shapes.
@gopherbot

Copy link
Copy Markdown
Contributor

This PR (HEAD: a7affd1) has been imported to Gerrit for code review.

Please visit Gerrit at https://go-review.googlesource.com/c/go/+/806101.

Important tips:

  • Don't comment on this PR. All discussion takes place in Gerrit.
  • You need a Gmail or other Google account to log in to Gerrit.
  • To change your code in response to feedback:
    • Push a new commit to the branch used by your GitHub PR.
    • A new "patch set" will then appear in Gerrit.
    • Respond to each comment by marking as Done in Gerrit if implemented as suggested. You can alternatively write a reply.
    • Critical: you must click the blue Reply button near the top to publish your Gerrit responses.
    • Multiple commits in the PR will be squashed by GerritBot.
  • The title and description of the GitHub PR are used to construct the final commit message.
    • Edit these as needed via the GitHub web interface (not via Gerrit or git).
    • You should word wrap the PR description at ~76 characters unless you need longer lines (e.g., for tables or URLs).
  • See the Sending a change via GitHub and Reviews sections of the Contribution Guide as well as the FAQ for details.

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Jorropo:

Patch Set 2:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Jorropo:

Patch Set 2: Commit-Queue+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com:

Patch Set 2:

Dry run: CV is trying the patch.

Bot data: {"action":"start","triggered_at":"2026-07-27T03:40:03Z","revision":"5328fd69e183d4d3ebd000433d8f5ff2117a6108"}


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from Jorropo:

Patch Set 2: -Commit-Queue

(Performed by <GERRIT_ACCOUNT_60063> on behalf of <GERRIT_ACCOUNT_55763>)


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com:

Patch Set 2:

This CL has passed the run


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

@gopherbot

Copy link
Copy Markdown
Contributor

Message from golang-scoped@luci-project-accounts.iam.gserviceaccount.com:

Patch Set 2: LUCI-TryBot-Result+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/806101.
After addressing review feedback, remember to publish your drafts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants