[go] cmd/compile: (amd64) optimize float32(round64(float64(x)))

5 views
Skip to first unread message

David Chase (Gerrit)

unread,
Dec 10, 2025, 4:11:47 PM (6 days ago) Dec 10
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

David Chase has uploaded the change for review

Commit message

cmd/compile: (amd64) optimize float32(round64(float64(x)))

Not a fix because there are other architectures
still to be done.

Updates #75463.
Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442

Change diff

diff --git a/src/cmd/compile/internal/amd64/ssa.go b/src/cmd/compile/internal/amd64/ssa.go
index 5ddcb84..c594673 100644
--- a/src/cmd/compile/internal/amd64/ssa.go
+++ b/src/cmd/compile/internal/amd64/ssa.go
@@ -1478,7 +1478,7 @@
 		}
 	case ssa.OpAMD64LoweredRound32F, ssa.OpAMD64LoweredRound64F:
 		// input is already rounded
-	case ssa.OpAMD64ROUNDSD:
+	case ssa.OpAMD64ROUNDSD, ssa.OpAMD64ROUNDSS:
 		p := s.Prog(v.Op.Asm())
 		val := v.AuxInt
 		// 0 means math.RoundToEven, 1 Floor, 2 Ceil, 3 Trunc
diff --git a/src/cmd/compile/internal/ssa/_gen/AMD64.rules b/src/cmd/compile/internal/ssa/_gen/AMD64.rules
index 353d272..4bee8fd 100644
--- a/src/cmd/compile/internal/ssa/_gen/AMD64.rules
+++ b/src/cmd/compile/internal/ssa/_gen/AMD64.rules
@@ -124,6 +124,8 @@
 (Ceil x)        => (ROUNDSD [2] x)
 (Trunc x)       => (ROUNDSD [3] x)
 
+(CVTSD2SS (ROUNDSD [c] (CVTSS2SD x))) => (ROUNDSS [c] x)
+
 (FMA x y z) => (VFMADD231SD z x y)
 
 // Lowering extension
diff --git a/src/cmd/compile/internal/ssa/_gen/AMD64Ops.go b/src/cmd/compile/internal/ssa/_gen/AMD64Ops.go
index 2fb4fdf..18ef7da 100644
--- a/src/cmd/compile/internal/ssa/_gen/AMD64Ops.go
+++ b/src/cmd/compile/internal/ssa/_gen/AMD64Ops.go
@@ -798,6 +798,7 @@
 		// ROUNDSD instruction is only guaraneteed to be available if GOAMD64>=v2.
 		// For GOAMD64<v2, any use must be preceded by a successful check of runtime.x86HasSSE41.
 		{name: "ROUNDSD", argLength: 1, reg: fp11, aux: "Int8", asm: "ROUNDSD"},
+		{name: "ROUNDSS", argLength: 1, reg: fp11, aux: "Int8", asm: "ROUNDSS"},
 		// See why we need those in issue #71204
 		{name: "LoweredRound32F", argLength: 1, reg: fp11, resultInArg0: true, zeroWidth: true},
 		{name: "LoweredRound64F", argLength: 1, reg: fp11, resultInArg0: true, zeroWidth: true},
diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go
index 00d581e..34e0344 100644
--- a/src/cmd/compile/internal/ssa/opGen.go
+++ b/src/cmd/compile/internal/ssa/opGen.go
@@ -929,6 +929,7 @@
 	OpAMD64SQRTSD
 	OpAMD64SQRTSS
 	OpAMD64ROUNDSD
+	OpAMD64ROUNDSS
 	OpAMD64LoweredRound32F
 	OpAMD64LoweredRound64F
 	OpAMD64VFMADD231SS
@@ -16239,6 +16240,20 @@
 		},
 	},
 	{
+		name:    "ROUNDSS",
+		auxType: auxInt8,
+		argLen:  1,
+		asm:     x86.AROUNDSS,
+		reg: regInfo{
+			inputs: []inputInfo{
+				{0, 2147418112}, // X0 X1 X2 X3 X4 X5 X6 X7 X8 X9 X10 X11 X12 X13 X14
+			},
+			outputs: []outputInfo{
+				{0, 2147418112}, // X0 X1 X2 X3 X4 X5 X6 X7 X8 X9 X10 X11 X12 X13 X14
+			},
+		},
+	},
+	{
 		name:         "LoweredRound32F",
 		argLen:       1,
 		resultInArg0: true,
diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go
index 19f16e1..0440cdb 100644
--- a/src/cmd/compile/internal/ssa/rewriteAMD64.go
+++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go
@@ -222,6 +222,8 @@
 		return rewriteValueAMD64_OpAMD64CMPXCHGLlock(v)
 	case OpAMD64CMPXCHGQlock:
 		return rewriteValueAMD64_OpAMD64CMPXCHGQlock(v)
+	case OpAMD64CVTSD2SS:
+		return rewriteValueAMD64_OpAMD64CVTSD2SS(v)
 	case OpAMD64DIVSD:
 		return rewriteValueAMD64_OpAMD64DIVSD(v)
 	case OpAMD64DIVSDload:
@@ -13513,6 +13515,27 @@
 	}
 	return false
 }
+func rewriteValueAMD64_OpAMD64CVTSD2SS(v *Value) bool {
+	v_0 := v.Args[0]
+	// match: (CVTSD2SS (ROUNDSD [c] (CVTSS2SD x)))
+	// result: (ROUNDSS [c] x)
+	for {
+		if v_0.Op != OpAMD64ROUNDSD {
+			break
+		}
+		c := auxIntToInt8(v_0.AuxInt)
+		v_0_0 := v_0.Args[0]
+		if v_0_0.Op != OpAMD64CVTSS2SD {
+			break
+		}
+		x := v_0_0.Args[0]
+		v.reset(OpAMD64ROUNDSS)
+		v.AuxInt = int8ToAuxInt(c)
+		v.AddArg(x)
+		return true
+	}
+	return false
+}
 func rewriteValueAMD64_OpAMD64DIVSD(v *Value) bool {
 	v_1 := v.Args[1]
 	v_0 := v.Args[0]

Change information

Files:
  • M src/cmd/compile/internal/amd64/ssa.go
  • M src/cmd/compile/internal/ssa/_gen/AMD64.rules
  • M src/cmd/compile/internal/ssa/_gen/AMD64Ops.go
  • M src/cmd/compile/internal/ssa/opGen.go
  • M src/cmd/compile/internal/ssa/rewriteAMD64.go
Change size: S
Delta: 5 files changed, 42 insertions(+), 1 deletion(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
Gerrit-Change-Number: 729160
Gerrit-PatchSet: 1
Gerrit-Owner: David Chase <drc...@google.com>
Gerrit-Reviewer: David Chase <drc...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Keith Randall (Gerrit)

unread,
Dec 11, 2025, 10:17:21 AM (6 days ago) Dec 11
to David Chase, goph...@pubsubhelper.golang.org, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
Attention needed from David Chase, Martin Möhrmann and Russ Cox

Keith Randall voted and added 1 comment

Votes added by Keith Randall

Code-Review+2

1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Keith Randall . resolved

It would be good to have a test for this.
float32(math.Round(float64(x)), for some interesting x. In particular, make sure that the round32 instruction has the same behavior near maxfloat32.

Open in Gerrit

Related details

Attention is currently required from:
  • David Chase
  • Martin Möhrmann
  • Russ Cox
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
Gerrit-Change-Number: 729160
Gerrit-PatchSet: 1
Gerrit-Owner: David Chase <drc...@google.com>
Gerrit-Reviewer: David Chase <drc...@google.com>
Gerrit-Reviewer: Keith Randall <k...@golang.org>
Gerrit-Reviewer: Martin Möhrmann <moeh...@google.com>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Russ Cox <r...@golang.org>
Gerrit-Attention: David Chase <drc...@google.com>
Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
Gerrit-Comment-Date: Thu, 11 Dec 2025 15:17:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Keith Randall (Gerrit)

unread,
Dec 11, 2025, 10:17:28 AM (6 days ago) Dec 11
to David Chase, goph...@pubsubhelper.golang.org, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
Attention needed from David Chase, Martin Möhrmann and Russ Cox

Keith Randall voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • David Chase
  • Martin Möhrmann
  • Russ Cox
Submit Requirements:
    • requirement satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
    Gerrit-Change-Number: 729160
    Gerrit-PatchSet: 1
    Gerrit-Owner: David Chase <drc...@google.com>
    Gerrit-Reviewer: David Chase <drc...@google.com>
    Gerrit-Reviewer: Keith Randall <k...@golang.org>
    Gerrit-Reviewer: Keith Randall <k...@google.com>
    Gerrit-Reviewer: Martin Möhrmann <moeh...@google.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: David Chase <drc...@google.com>
    Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
    Gerrit-Comment-Date: Thu, 11 Dec 2025 15:17:22 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    David Chase (Gerrit)

    unread,
    Dec 15, 2025, 12:31:44 PM (2 days ago) Dec 15
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from David Chase, Keith Randall, Keith Randall, Martin Möhrmann and Russ Cox

    David Chase uploaded new patchset

    David Chase uploaded patch set #2 to this change.
    Following approvals got outdated and were removed:
    • Code-Review: +2 by Keith Randall, +1 by Keith Randall
    • TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Chase
    • Keith Randall
    • Keith Randall
    • Martin Möhrmann
    • Russ Cox
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedNo-Wait-Release
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
    Gerrit-Change-Number: 729160
    Gerrit-PatchSet: 2
    Gerrit-Owner: David Chase <drc...@google.com>
    Gerrit-Reviewer: David Chase <drc...@google.com>
    Gerrit-Reviewer: Keith Randall <k...@golang.org>
    Gerrit-Reviewer: Keith Randall <k...@google.com>
    Gerrit-Reviewer: Martin Möhrmann <moeh...@google.com>
    Gerrit-Reviewer: Russ Cox <r...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Keith Randall <k...@golang.org>
    Gerrit-Attention: Russ Cox <r...@golang.org>
    Gerrit-Attention: David Chase <drc...@google.com>
    Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
    Gerrit-Attention: Keith Randall <k...@google.com>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Keith Randall (Gerrit)

    unread,
    Dec 16, 2025, 1:00:43 PM (13 hours ago) Dec 16
    to David Chase, goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from David Chase, Keith Randall, Martin Möhrmann and Russ Cox

    Keith Randall added 1 comment

    Patchset-level comments
    File-level comment, Patchset 1:
    Keith Randall . unresolved

    It would be good to have a test for this.
    float32(math.Round(float64(x)), for some interesting x. In particular, make sure that the round32 instruction has the same behavior near maxfloat32.

    Keith Randall

    I want a correctness test, not a codegen test (although a codegen test is good to have). Just to make sure that our rewrite rules are faithfully transferring the rounding direction to the 32-bit versions.
    Or do we already have such a test somewhere?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • David Chase
    • Keith Randall
    • Martin Möhrmann
    • Russ Cox
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedNo-Wait-Release
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
      Gerrit-Change-Number: 729160
      Gerrit-PatchSet: 2
      Gerrit-Owner: David Chase <drc...@google.com>
      Gerrit-Reviewer: David Chase <drc...@google.com>
      Gerrit-Reviewer: Keith Randall <k...@golang.org>
      Gerrit-Reviewer: Keith Randall <k...@google.com>
      Gerrit-Reviewer: Martin Möhrmann <moeh...@google.com>
      Gerrit-Reviewer: Russ Cox <r...@golang.org>
      Gerrit-CC: Gopher Robot <go...@golang.org>
      Gerrit-Attention: Russ Cox <r...@golang.org>
      Gerrit-Attention: David Chase <drc...@google.com>
      Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
      Gerrit-Attention: Keith Randall <k...@google.com>
      Gerrit-Comment-Date: Tue, 16 Dec 2025 18:00:34 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Keith Randall <k...@golang.org>
      unsatisfied_requirement
      open
      diffy

      David Chase (Gerrit)

      unread,
      Dec 16, 2025, 3:47:24 PM (10 hours ago) Dec 16
      to goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
      Attention needed from Keith Randall, Keith Randall, Martin Möhrmann and Russ Cox

      David Chase added 1 comment

      Patchset-level comments
      Keith Randall . unresolved

      It would be good to have a test for this.
      float32(math.Round(float64(x)), for some interesting x. In particular, make sure that the round32 instruction has the same behavior near maxfloat32.

      Keith Randall

      I want a correctness test, not a codegen test (although a codegen test is good to have). Just to make sure that our rewrite rules are faithfully transferring the rounding direction to the 32-bit versions.
      Or do we already have such a test somewhere?

      David Chase

      I don't think we have that test, I just haven't gotten around to the correctness and corner case tests. I *think* that there are cases where this optimization could do one fewer rounding and maybe change answers, but it would be fewer roudings and hence "better", and since we don't give any other way to express the 32-bit roundings I would normally assume that someone wants the better answer.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Keith Randall
      Gerrit-Attention: Keith Randall <k...@golang.org>
      Gerrit-Attention: Russ Cox <r...@golang.org>
      Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
      Gerrit-Attention: Keith Randall <k...@google.com>
      Gerrit-Comment-Date: Tue, 16 Dec 2025 20:47:21 +0000
      unsatisfied_requirement
      open
      diffy

      David Chase (Gerrit)

      unread,
      Dec 16, 2025, 5:03:36 PM (9 hours ago) Dec 16
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
      Attention needed from Keith Randall, Keith Randall, Martin Möhrmann and Russ Cox

      David Chase uploaded new patchset

      David Chase uploaded patch set #3 to this change.
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Keith Randall
      • Keith Randall
      • Martin Möhrmann
      • Russ Cox
      Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedNo-Wait-Release
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
      Gerrit-Change-Number: 729160
      Gerrit-PatchSet: 3
      unsatisfied_requirement
      open
      diffy

      David Chase (Gerrit)

      unread,
      Dec 16, 2025, 6:21:06 PM (8 hours ago) Dec 16
      to goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
      Attention needed from Keith Randall, Keith Randall, Martin Möhrmann and Russ Cox

      David Chase added 1 comment

      Patchset-level comments
      File-level comment, Patchset 1:
      Keith Randall . resolved

      It would be good to have a test for this.
      float32(math.Round(float64(x)), for some interesting x. In particular, make sure that the round32 instruction has the same behavior near maxfloat32.

      Keith Randall

      I want a correctness test, not a codegen test (although a codegen test is good to have). Just to make sure that our rewrite rules are faithfully transferring the rounding direction to the 32-bit versions.
      Or do we already have such a test somewhere?

      David Chase

      I don't think we have that test, I just haven't gotten around to the correctness and corner case tests. I *think* that there are cases where this optimization could do one fewer rounding and maybe change answers, but it would be fewer roudings and hence "better", and since we don't give any other way to express the 32-bit roundings I would normally assume that someone wants the better answer.

      David Chase

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Keith Randall
      • Keith Randall
      • Martin Möhrmann
      • Russ Cox
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedNo-Wait-Release
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I3d7754ce4a26af0f5c4ef0be1254d164e68f8442
        Gerrit-Change-Number: 729160
        Gerrit-PatchSet: 3
        Gerrit-Owner: David Chase <drc...@google.com>
        Gerrit-Reviewer: David Chase <drc...@google.com>
        Gerrit-Reviewer: Keith Randall <k...@golang.org>
        Gerrit-Reviewer: Keith Randall <k...@google.com>
        Gerrit-Reviewer: Martin Möhrmann <moeh...@google.com>
        Gerrit-Reviewer: Russ Cox <r...@golang.org>
        Gerrit-CC: Gopher Robot <go...@golang.org>
        Gerrit-Attention: Keith Randall <k...@golang.org>
        Gerrit-Attention: Russ Cox <r...@golang.org>
        Gerrit-Attention: Martin Möhrmann <moeh...@google.com>
        Gerrit-Attention: Keith Randall <k...@google.com>
        Gerrit-Comment-Date: Tue, 16 Dec 2025 23:21:03 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Keith Randall <k...@golang.org>
        Comment-In-Reply-To: David Chase <drc...@google.com>
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy

        David Chase (Gerrit)

        unread,
        Dec 16, 2025, 6:21:13 PM (8 hours ago) Dec 16
        to goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Go LUCI, Martin Möhrmann, Russ Cox, Gopher Robot, golang-co...@googlegroups.com
        Attention needed from Keith Randall, Keith Randall, Martin Möhrmann and Russ Cox

        David Chase voted Commit-Queue+1

        Commit-Queue+1
        Gerrit-Comment-Date: Tue, 16 Dec 2025 23:21:10 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        unsatisfied_requirement
        satisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages