Skip to content

Commit 1a0b863

Browse files
committed
cmd/compile: remove redundant calls to cmpstring
The results of cmpstring are reuseable if the second call has the same arguments and memory. Note that this gets rid of cmpstring, but we still generate a redundant </<= test and branch afterwards, because the compiler doesn't know that cmpstring only ever returns -1,0,1. Update #61725 Change-Id: I93a0d1ccca50d90b1e1a888240ffb75a3b10b59b Reviewed-on: https://go-review.googlesource.com/c/go/+/578835 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
1 parent d428a63 commit 1a0b863

File tree

4 files changed

+74
-5
lines changed

4 files changed

+74
-5
lines changed

src/cmd/compile/internal/ssa/_gen/generic.rules

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2794,3 +2794,12 @@
27942794
(Load <t> (OffPtr [off] (Convert (Addr {sym} _) _) ) _) && t.IsInteger() && t.Size() == 4 && isFixed32(config, sym, off) => (Const32 [fixed32(config, sym, off)])
27952795
(Load <t> (OffPtr [off] (ITab (IMake (Addr {sym} _) _))) _) && t.IsInteger() && t.Size() == 4 && isFixed32(config, sym, off) => (Const32 [fixed32(config, sym, off)])
27962796
(Load <t> (OffPtr [off] (ITab (IMake (Convert (Addr {sym} _) _) _))) _) && t.IsInteger() && t.Size() == 4 && isFixed32(config, sym, off) => (Const32 [fixed32(config, sym, off)])
2797+
2798+
// Calling cmpstring a second time with the same arguments in the
2799+
// same memory state can reuse the results of the first call.
2800+
// See issue 61725.
2801+
// Note that this could pretty easily generalize to any pure function.
2802+
(StaticLECall {f} x y m:(SelectN [1] c:(StaticLECall {g} x y mem)))
2803+
&& isSameCall(f, "runtime.cmpstring")
2804+
&& isSameCall(g, "runtime.cmpstring")
2805+
=> (MakeResult (SelectN [0] <typ.Int> c) m)

src/cmd/compile/internal/ssa/rewritegeneric.go

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cmp/cmp.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,19 @@ func Less[T Ordered](x, y T) bool {
4040
func Compare[T Ordered](x, y T) int {
4141
xNaN := isNaN(x)
4242
yNaN := isNaN(y)
43-
if xNaN && yNaN {
44-
return 0
43+
if xNaN {
44+
if yNaN {
45+
return 0
46+
}
47+
return -1
48+
}
49+
if yNaN {
50+
return +1
4551
}
46-
if xNaN || x < y {
52+
if x < y {
4753
return -1
4854
}
49-
if yNaN || x > y {
55+
if x > y {
5056
return +1
5157
}
5258
return 0

test/codegen/comparisons.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
package codegen
88

9-
import "unsafe"
9+
import (
10+
"cmp"
11+
"unsafe"
12+
)
1013

1114
// This file contains code generation tests related to the comparison
1215
// operators.
@@ -801,3 +804,24 @@ func invertLessThanNoov(p1, p2, p3 Point) bool {
801804
// arm64:`CMP`,`CSET`,`CSEL`
802805
return (p1.X-p3.X)*(p2.Y-p3.Y)-(p2.X-p3.X)*(p1.Y-p3.Y) < 0
803806
}
807+
808+
func cmpstring1(x, y string) int {
809+
// amd64:".*cmpstring"
810+
if x < y {
811+
return -1
812+
}
813+
// amd64:-".*cmpstring"
814+
if x > y {
815+
return +1
816+
}
817+
return 0
818+
}
819+
func cmpstring2(x, y string) int {
820+
// We want to fail if there are two calls to cmpstring.
821+
// They will both have the same line number, so a test
822+
// like in cmpstring1 will not work. Instead, we
823+
// look for spill/restore instructions, which only
824+
// need to exist if there are 2 calls.
825+
//amd64:-`MOVQ\t.*\(SP\)`
826+
return cmp.Compare(x, y)
827+
}

0 commit comments

Comments
 (0)