Skip to content

Commit 1d3f49f

Browse files
committed
cmd/compile: when inlining ==, don’t take the address of the values
DO NOT REVIEW [There is no clear way to improve things for all backends. So this CL is going to wait until we are SSA by default everywhere.] This CL reworks walkcompare for clarity and concision. It also makes one significant functional change. (The functional change is hard to separate cleanly from the cleanup, so I just did them together.) When inlining and unrolling an equality comparison for a small struct or array, compare the elements like: a[0] == b[0] && a[1] == b[1] rather than pa := &a pb := &b pa[0] == pb[0] && pa[1] == pb[1] The result is the same, but taking the address and working through the indirect forces the backends to generate less efficient code. Updates golang#15303 Sample code: type T struct { a, b int8 } func g(a T) bool { return a == T{1, 2} } SSA before: "".g t=1 size=80 args=0x10 locals=0x8 0x0000 00000 (badeq.go:7) TEXT "".g(SB), $8-16 0x0000 00000 (badeq.go:7) SUBQ $8, SP 0x0004 00004 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB) 0x0004 00004 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0004 00004 (badeq.go:8) MOVBLZX "".a+16(FP), AX 0x0009 00009 (badeq.go:8) MOVB AL, "".autotmp_0+6(SP) 0x000d 00013 (badeq.go:8) MOVBLZX "".a+17(FP), AX 0x0012 00018 (badeq.go:8) MOVB AL, "".autotmp_0+7(SP) 0x0016 00022 (badeq.go:8) MOVB $0, "".autotmp_1+4(SP) 0x001b 00027 (badeq.go:8) MOVB $1, "".autotmp_1+4(SP) 0x0020 00032 (badeq.go:8) MOVB $2, "".autotmp_1+5(SP) 0x0025 00037 (badeq.go:8) MOVBLZX "".autotmp_0+6(SP), AX 0x002a 00042 (badeq.go:8) MOVBLZX "".autotmp_1+4(SP), CX 0x002f 00047 (badeq.go:8) CMPB AL, CL 0x0031 00049 (badeq.go:8) JNE 70 0x0033 00051 (badeq.go:8) MOVBLZX "".autotmp_0+7(SP), AX 0x0038 00056 (badeq.go:8) CMPB AL, $2 0x003a 00058 (badeq.go:8) SETEQ AL 0x003d 00061 (badeq.go:8) MOVB AL, "".~r1+24(FP) 0x0041 00065 (badeq.go:8) ADDQ $8, SP 0x0045 00069 (badeq.go:8) RET 0x0046 00070 (badeq.go:8) MOVB $0, AL 0x0048 00072 (badeq.go:8) JMP 61 SSA after: "".g t=1 size=32 args=0x10 locals=0x0 0x0000 00000 (badeq.go:7) TEXT "".g(SB), $0-16 0x0000 00000 (badeq.go:7) NOP 0x0000 00000 (badeq.go:7) NOP 0x0000 00000 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB) 0x0000 00000 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0000 00000 (badeq.go:8) MOVBLZX "".a+8(FP), AX 0x0005 00005 (badeq.go:8) CMPB AL, $1 0x0007 00007 (badeq.go:8) JNE 25 0x0009 00009 (badeq.go:8) MOVBLZX "".a+9(FP), CX 0x000e 00014 (badeq.go:8) CMPB CL, $2 0x0011 00017 (badeq.go:8) SETEQ AL 0x0014 00020 (badeq.go:8) MOVB AL, "".~r1+16(FP) 0x0018 00024 (badeq.go:8) RET 0x0019 00025 (badeq.go:8) MOVB $0, AL 0x001b 00027 (badeq.go:8) JMP 20 Old backend before: "".g t=1 size=96 args=0x10 locals=0x8 0x0000 00000 (badeq.go:7) TEXT "".g(SB), $8-16 0x0000 00000 (badeq.go:7) SUBQ $8, SP 0x0004 00004 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB) 0x0004 00004 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0004 00004 (badeq.go:8) MOVBQZX "".a+16(FP), BX 0x0009 00009 (badeq.go:8) MOVB BL, "".autotmp_0+6(SP) 0x000d 00013 (badeq.go:8) MOVBQZX "".a+17(FP), BX 0x0012 00018 (badeq.go:8) MOVB BL, "".autotmp_0+7(SP) 0x0016 00022 (badeq.go:8) MOVQ $0, CX 0x0018 00024 (badeq.go:8) MOVB CL, "".autotmp_1+4(SP) 0x001c 00028 (badeq.go:8) MOVB CL, "".autotmp_1+5(SP) 0x0020 00032 (badeq.go:8) MOVB $1, "".autotmp_1+4(SP) 0x0025 00037 (badeq.go:8) MOVB $2, "".autotmp_1+5(SP) 0x002a 00042 (badeq.go:8) LEAQ "".autotmp_0+6(SP), CX 0x002f 00047 (badeq.go:8) LEAQ "".autotmp_1+4(SP), BX 0x0034 00052 (badeq.go:8) MOVQ BX, AX 0x0037 00055 (badeq.go:8) NOP 0x0037 00055 (badeq.go:8) MOVBQZX (CX), BX 0x003a 00058 (badeq.go:8) NOP 0x003a 00058 (badeq.go:8) MOVBQZX (AX), BP 0x003d 00061 (badeq.go:8) CMPB BL, BPB 0x0040 00064 (badeq.go:8) JNE 87 0x0042 00066 (badeq.go:8) NOP 0x0042 00066 (badeq.go:8) MOVBQZX 1(CX), BX 0x0046 00070 (badeq.go:8) NOP 0x0046 00070 (badeq.go:8) MOVBQZX 1(AX), BP 0x004a 00074 (badeq.go:8) CMPB BL, BPB 0x004d 00077 (badeq.go:8) SETEQ "".~r1+24(FP) 0x0052 00082 (badeq.go:8) ADDQ $8, SP 0x0056 00086 (badeq.go:8) RET 0x0057 00087 (badeq.go:8) MOVB $0, "".~r1+24(FP) 0x005c 00092 (badeq.go:8) JMP 82 Old backend after: "".g t=1 size=64 args=0x10 locals=0x0 0x0000 00000 (badeq.go:7) TEXT "".g(SB), $0-16 0x0000 00000 (badeq.go:7) NOP 0x0000 00000 (badeq.go:7) NOP 0x0000 00000 (badeq.go:7) FUNCDATA $0, gclocals·23e8278e2b69a3a75fa59b23c49ed6ad(SB) 0x0000 00000 (badeq.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB) 0x0000 00000 (badeq.go:8) NOP 0x0000 00000 (badeq.go:8) MOVBQZX "".a+8(FP), BX 0x0005 00005 (badeq.go:8) MOVQ BX, BP 0x0008 00008 (badeq.go:8) MOVBQZX "".a+9(FP), BX 0x000d 00013 (badeq.go:8) MOVQ BX, DX 0x0010 00016 (badeq.go:8) NOP 0x0010 00016 (badeq.go:8) MOVQ $0, BX 0x0012 00018 (badeq.go:8) NOP 0x0012 00018 (badeq.go:8) MOVQ $1, CX 0x0019 00025 (badeq.go:8) MOVQ $2, AX 0x0020 00032 (badeq.go:8) CMPB BPB, CL 0x0023 00035 (badeq.go:8) JNE 45 0x0025 00037 (badeq.go:8) CMPB DL, AL 0x0027 00039 (badeq.go:8) SETEQ "".~r1+16(FP) 0x002c 00044 (badeq.go:8) RET 0x002d 00045 (badeq.go:8) MOVB $0, "".~r1+16(FP) 0x0032 00050 (badeq.go:8) JMP 44 Change-Id: I120185d58012b7bbcdb1ec01225b5b08d0855d86
1 parent b859a78 commit 1d3f49f

File tree

2 files changed

+79
-89
lines changed

2 files changed

+79
-89
lines changed

src/cmd/compile/internal/gc/walk.go

Lines changed: 55 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -3083,9 +3083,7 @@ func walkcompare(n *Node, init *Nodes) *Node {
30833083
// Handle != similarly.
30843084
// This avoids the allocation that would be required
30853085
// to convert r to l for comparison.
3086-
var l *Node
3087-
3088-
var r *Node
3086+
var l, r *Node
30893087
if n.Left.Type.IsInterface() && !n.Right.Type.IsInterface() {
30903088
l = n.Left
30913089
r = n.Right
@@ -3129,14 +3127,17 @@ func walkcompare(n *Node, init *Nodes) *Node {
31293127

31303128
// Must be comparison of array or struct.
31313129
// Otherwise back end handles it.
3130+
// While we're here, decide whether to
3131+
// inline or call an eq alg.
31323132
t := n.Left.Type
3133-
3133+
var inline bool
31343134
switch t.Etype {
31353135
default:
31363136
return n
3137-
3138-
case TARRAY, TSTRUCT:
3139-
break
3137+
case TARRAY:
3138+
inline = t.NumElem() <= 1 || (t.NumElem() <= 4 && issimple[t.Elem().Etype])
3139+
case TSTRUCT:
3140+
inline = t.NumFields() <= 4
31403141
}
31413142

31423143
cmpl := n.Left
@@ -3152,103 +3153,68 @@ func walkcompare(n *Node, init *Nodes) *Node {
31523153
Fatalf("arguments of comparison must be lvalues - %v %v", cmpl, cmpr)
31533154
}
31543155

3155-
l = temp(Ptrto(t))
3156-
a := Nod(OAS, l, Nod(OADDR, cmpl, nil))
3157-
a.Right.Etype = 1 // addr does not escape
3158-
a = typecheck(a, Etop)
3159-
init.Append(a)
3160-
3161-
r = temp(Ptrto(t))
3162-
a = Nod(OAS, r, Nod(OADDR, cmpr, nil))
3163-
a.Right.Etype = 1 // addr does not escape
3164-
a = typecheck(a, Etop)
3165-
init.Append(a)
3156+
// Chose not to inline. Call equality function directly.
3157+
if !inline {
3158+
// eq algs take pointers
3159+
pl := temp(Ptrto(t))
3160+
al := Nod(OAS, pl, Nod(OADDR, cmpl, nil))
3161+
al.Right.Etype = 1 // addr does not escape
3162+
al = typecheck(al, Etop)
3163+
init.Append(al)
3164+
3165+
pr := temp(Ptrto(t))
3166+
ar := Nod(OAS, pr, Nod(OADDR, cmpr, nil))
3167+
ar.Right.Etype = 1 // addr does not escape
3168+
ar = typecheck(ar, Etop)
3169+
init.Append(ar)
3170+
3171+
var needsize int
3172+
call := Nod(OCALL, eqfor(t, &needsize), nil)
3173+
call.List.Append(pl)
3174+
call.List.Append(pr)
3175+
if needsize != 0 {
3176+
call.List.Append(Nodintconst(t.Width))
3177+
}
3178+
res := call
3179+
if n.Op != OEQ {
3180+
res = Nod(ONOT, res, nil)
3181+
}
3182+
n = finishcompare(n, res, init)
3183+
return n
3184+
}
31663185

3167-
var andor Op = OANDAND
3186+
// inline: build boolean expression comparing element by element
3187+
andor := OANDAND
31683188
if n.Op == ONE {
31693189
andor = OOROR
31703190
}
3171-
31723191
var expr *Node
3173-
if t.Etype == TARRAY && t.NumElem() <= 4 && issimple[t.Elem().Etype] {
3174-
// Four or fewer elements of a basic type.
3175-
// Unroll comparisons.
3176-
var li *Node
3177-
var ri *Node
3178-
for i := 0; int64(i) < t.NumElem(); i++ {
3179-
li = Nod(OINDEX, l, Nodintconst(int64(i)))
3180-
ri = Nod(OINDEX, r, Nodintconst(int64(i)))
3181-
a = Nod(n.Op, li, ri)
3182-
if expr == nil {
3183-
expr = a
3184-
} else {
3185-
expr = Nod(andor, expr, a)
3186-
}
3187-
}
3188-
3192+
compare := func(el, er *Node) {
3193+
a := Nod(n.Op, el, er)
31893194
if expr == nil {
3190-
expr = Nodbool(n.Op == OEQ)
3191-
}
3192-
n = finishcompare(n, expr, init)
3193-
return n
3194-
}
3195-
3196-
if t.Etype == TARRAY {
3197-
// Zero- or single-element array, of any type.
3198-
switch t.NumElem() {
3199-
case 0:
3200-
n = finishcompare(n, Nodbool(n.Op == OEQ), init)
3201-
return n
3202-
case 1:
3203-
l0 := Nod(OINDEX, l, Nodintconst(0))
3204-
r0 := Nod(OINDEX, r, Nodintconst(0))
3205-
a := Nod(n.Op, l0, r0)
3206-
n = finishcompare(n, a, init)
3207-
return n
3195+
expr = a
3196+
} else {
3197+
expr = Nod(andor, expr, a)
32083198
}
32093199
}
3210-
3211-
if t.IsStruct() && t.NumFields() <= 4 {
3212-
// Struct of four or fewer fields.
3213-
// Inline comparisons.
3214-
var li *Node
3215-
var ri *Node
3200+
if t.IsStruct() {
32163201
for _, t1 := range t.Fields().Slice() {
3217-
if isblanksym(t1.Sym) {
3202+
sym := t1.Sym
3203+
if isblanksym(sym) {
32183204
continue
32193205
}
3220-
li = NodSym(OXDOT, l, t1.Sym)
3221-
ri = NodSym(OXDOT, r, t1.Sym)
3222-
a = Nod(n.Op, li, ri)
3223-
if expr == nil {
3224-
expr = a
3225-
} else {
3226-
expr = Nod(andor, expr, a)
3227-
}
3206+
compare(NodSym(OXDOT, cmpl, sym), NodSym(OXDOT, cmpr, sym))
32283207
}
3229-
3230-
if expr == nil {
3231-
expr = Nodbool(n.Op == OEQ)
3208+
} else {
3209+
for i := 0; int64(i) < t.NumElem(); i++ {
3210+
idx := Nodintconst(int64(i))
3211+
compare(Nod(OINDEX, cmpl, idx), Nod(OINDEX, cmpr, idx))
32323212
}
3233-
n = finishcompare(n, expr, init)
3234-
return n
32353213
}
3236-
3237-
// Chose not to inline. Call equality function directly.
3238-
var needsize int
3239-
call := Nod(OCALL, eqfor(t, &needsize), nil)
3240-
3241-
call.List.Append(l)
3242-
call.List.Append(r)
3243-
if needsize != 0 {
3244-
call.List.Append(Nodintconst(t.Width))
3214+
if expr == nil {
3215+
expr = Nodbool(n.Op == OEQ)
32453216
}
3246-
r = call
3247-
if n.Op != OEQ {
3248-
r = Nod(ONOT, r, nil)
3249-
}
3250-
3251-
n = finishcompare(n, r, init)
3217+
n = finishcompare(n, expr, init)
32523218
return n
32533219
}
32543220

test/fixedbugs/issue15303.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// run
2+
3+
// Copyright 2016 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Ensure that inlined struct/array comparisons have the right side-effects.
8+
9+
package main
10+
11+
import "os"
12+
13+
func main() {
14+
var x int
15+
f := func() (r [4]int) {
16+
x++
17+
return
18+
}
19+
_ = f() == f()
20+
if x != 2 {
21+
println("f evaluated ", x, " times, want 2")
22+
os.Exit(1)
23+
}
24+
}

0 commit comments

Comments
 (0)