[go] cmd/compile: remove flags → bool → flags roundtrips on amd64

0 views

Skip to first unread message

Gopher Robot (Gerrit)

unread,

May 15, 2026, 7:23:32 PM (10 hours ago) May 15

to Jorropo, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, golang...@luci-project-accounts.iam.gserviceaccount.com, Dmitri Shuralyov, Dmitri Shuralyov, Keith Randall, Keith Randall, Martin Möhrmann, golang-co...@googlegroups.com

Gopher Robot submitted the change with unreviewed changes

Unreviewed changes

4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/cmd/compile/internal/ssa/_gen/AMD64.rules
Insertions: 1, Deletions: 1.

@@ -1814,7 +1814,7 @@
 	(VPMOVMToVec64x8  (VCMPPD512 [3] x y))
 
 // remove flags → bool → flags roundtrip
-// Only do it if the flag generating instruction is local otherwise the likely hood flagalloc wont undo this optimization and makes things worst are slim.
+// Only do it if the flag generating instruction is local otherwise the likelihood flagalloc won't undo this optimization and makes things worse are slim.
 (NE t:(TESTQ x:(MOVBQZX s:(SET(EQ|NE|L|G|LE|GE|A|B|AE|BE|EQF|NEF|GF|GEF) flags)) x) yes no) && t.Block == s.Block => ((EQ|NE|LT|GT|LE|GE|UGT|ULT|UGE|ULE|EQF|NEF|UGE|UGT) flags yes no)
 (NE t:(TESTL x:(MOVBQZX s:(SET(EQ|NE|L|G|LE|GE|A|B|AE|BE|EQF|NEF|GF|GEF) flags)) x) yes no) && t.Block == s.Block => ((EQ|NE|LT|GT|LE|GE|UGT|ULT|UGE|ULE|EQF|NEF|UGE|UGT) flags yes no)
 (NE t:(TESTW x:(MOVBQZX s:(SET(EQ|NE|L|G|LE|GE|A|B|AE|BE|EQF|NEF|GF|GEF) flags)) x) yes no) && t.Block == s.Block => ((EQ|NE|LT|GT|LE|GE|UGT|ULT|UGE|ULE|EQF|NEF|UGE|UGT) flags yes no)
```

Change information

Commit message:

cmd/compile: remove flags → bool → flags roundtrips on amd64

Fixes #76056
Fixes #76060

If we modify the issue's fieldReduceOnce2 function to:

  // fieldReduceOnce reduces a value a < 2q.
  func fieldReduceOnce2(a uint32) fieldElement {
    x, b := bits.Sub(uint(a), uint(q), 0)
    return fieldElement(subtle.ConstantTimeSelect(int(b), int(a), int(x)))
  }

We get the wanted assembly*:
  MOVL AX, CX
  MOVL AX, DX
  SUBQ $8380417, CX
  CMOVQCS DX, CX
  MOVQ CX, AX ; not ideal code size but handled by the register renaming unit
  RET

Changes made to fieldReduceOnce2:
- fixed a bug where a and x arguments to subtle.ConstantTimeSelect were swapped.
  we should use a when the sub underflows and x otherwise.
- use bits.Sub rather than bits.Sub32 which is intriscified.

*we use CMOVQCS + MOVQ because the CMOV randomly gets generated backward,
I believe this would be fixed if we teach regalloc to commut CMOV
(by swapping the two register args and inverting the condition).

Change-Id: I01eca545d3c5c8a1c1f5a107e0089f715359dfc6

Reviewed-on: https://go-review.googlesource.com/c/go/+/778141

LUCI-TryBot-Result: golang...@luci-project-accounts.iam.gserviceaccount.com <golang...@luci-project-accounts.iam.gserviceaccount.com>

Reviewed-by: Keith Randall <k...@google.com>

Auto-Submit: Jorropo <jorro...@gmail.com>

Reviewed-by: Keith Randall <k...@golang.org>

Reviewed-by: Dmitri Shuralyov <dmit...@google.com>

Files:

M src/cmd/compile/internal/ssa/_gen/AMD64.rules
M src/cmd/compile/internal/ssa/prove.go
M src/cmd/compile/internal/ssa/rewriteAMD64.go
M test/codegen/condmove.go

Change size: XL

Delta: 4 files changed, 14087 insertions(+), 10 deletions(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +1 by Keith Randall, +2 by Keith Randall, +1 by Dmitri Shuralyov
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

0 new messages