Skip to content

Commit eaabb84

Browse files
committed
cmd/gc: evaluate concrete == interface without allocating
Consider an interface value i of type I and concrete value c of type C. Prior to this CL, i==c was evaluated as I(c) == i As of Go 1.4, all interfaces hold pointers, so evaluating I(c) allocates. This CL changes the evaluation of i==c to x, ok := i.(C); ok && x == c The new generated code is shorter, uses less stack space, does not allocate, and makes one runtime call instead of two. This kind of comparison occurs in 38 places in the stdlib, mostly in the net and os packages. benchmark old ns/op new ns/op delta BenchmarkCmpIfaceConcrete 31.6 7.92 -74.94% BenchmarkCmpEfaceConcrete 29.7 7.91 -73.37% Fixes golang#9370. Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
1 parent c7eb966 commit eaabb84

File tree

5 files changed

+151
-18
lines changed

5 files changed

+151
-18
lines changed

src/cmd/gc/typecheck.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -575,27 +575,32 @@ typecheck1(Node **np, int top)
575575
// the same type.
576576
//
577577
// the only conversion that isn't a no-op is concrete == interface.
578-
// in that case, check comparability of the concrete type.
578+
// in that case, check comparability of the concrete type,
579+
// but do not do the actual conversion here, to avoid allocating.
579580
if(r->type->etype != TBLANK && (aop = assignop(l->type, r->type, nil)) != 0) {
580581
if(isinter(r->type) && !isinter(l->type) && algtype1(l->type, nil) == ANOEQ) {
581582
yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(l->type));
582583
goto error;
583584
}
584-
l = nod(aop, l, N);
585-
l->type = r->type;
586-
l->typecheck = 1;
587-
n->left = l;
588-
t = l->type;
585+
if(isinter(r->type) == isinter(l->type)) {
586+
l = nod(aop, l, N);
587+
l->type = r->type;
588+
l->typecheck = 1;
589+
n->left = l;
590+
}
591+
t = r->type;
589592
} else if(l->type->etype != TBLANK && (aop = assignop(r->type, l->type, nil)) != 0) {
590593
if(isinter(l->type) && !isinter(r->type) && algtype1(r->type, nil) == ANOEQ) {
591594
yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(r->type));
592595
goto error;
593596
}
594-
r = nod(aop, r, N);
595-
r->type = l->type;
596-
r->typecheck = 1;
597-
n->right = r;
598-
t = r->type;
597+
if(isinter(r->type) == isinter(l->type)) {
598+
r = nod(aop, r, N);
599+
r->type = l->type;
600+
r->typecheck = 1;
601+
n->right = r;
602+
}
603+
t = l->type;
599604
}
600605
et = t->etype;
601606
}
@@ -605,8 +610,10 @@ typecheck1(Node **np, int top)
605610
yyerror("invalid operation: %N (non-numeric type %T)", n, l->type);
606611
goto error;
607612
}
608-
yyerror("invalid operation: %N (mismatched types %T and %T)", n, l->type, r->type);
609-
goto error;
613+
if(isinter(r->type) == isinter(l->type) || aop == 0) {
614+
yyerror("invalid operation: %N (mismatched types %T and %T)", n, l->type, r->type);
615+
goto error;
616+
}
610617
}
611618
if(!okfor[op][et]) {
612619
yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(t));
@@ -674,7 +681,7 @@ typecheck1(Node **np, int top)
674681
n->right = l;
675682
} else if(r->op == OLITERAL && r->val.ctype == CTNIL) {
676683
// leave alone for back end
677-
} else {
684+
} else if(isinter(r->type) == isinter(l->type)) {
678685
n->etype = n->op;
679686
n->op = OCMPIFACE;
680687
}

src/cmd/gc/walk.c

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3237,10 +3237,54 @@ static void
32373237
walkcompare(Node **np, NodeList **init)
32383238
{
32393239
Node *n, *l, *r, *call, *a, *li, *ri, *expr, *cmpl, *cmpr;
3240+
Node *x, *ok;
32403241
int andor, i;
32413242
Type *t, *t1;
32423243

32433244
n = *np;
3245+
3246+
// Given interface value l and concrete value r, rewrite
3247+
// l == r
3248+
// to
3249+
// x, ok := l.(type(r)); ok && x == r
3250+
// Handle != similarly.
3251+
// This avoids the allocation that would be required
3252+
// to convert r to l for comparison.
3253+
l = N;
3254+
r = N;
3255+
if(isinter(n->left->type) && !isinter(n->right->type)) {
3256+
l = n->left;
3257+
r = n->right;
3258+
} else if(!isinter(n->left->type) && isinter(n->right->type)) {
3259+
l = n->right;
3260+
r = n->left;
3261+
}
3262+
if(l != N) {
3263+
x = temp(r->type);
3264+
x->typecheck = 1;
3265+
ok = temp(types[TBOOL]);
3266+
ok->typecheck = 1;
3267+
3268+
// l.(type(r))
3269+
a = nod(ODOTTYPE2, l, N);
3270+
a->type = r->type;
3271+
a->typecheck = 1;
3272+
3273+
// x, ok := l.(type(r))
3274+
expr = nod(OAS2DOTTYPE, N, N);
3275+
expr->list = list1(x);
3276+
expr->list = list(expr->list, ok);
3277+
expr->rlist = list1(a);
3278+
expr->typecheck = 1;
3279+
walkexpr(&expr, init);
3280+
3281+
if(n->op == OEQ)
3282+
r = nod(OANDAND, ok, nod(OEQ, x, r));
3283+
else
3284+
r = nod(OOROR, nod(ONOT, ok, N), nod(ONE, x, r));
3285+
*init = list(*init, expr);
3286+
goto ret;
3287+
}
32443288

32453289
// Must be comparison of array or struct.
32463290
// Otherwise back end handles it.

src/runtime/iface_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package runtime_test
66

77
import (
8+
"runtime"
89
"testing"
910
)
1011

@@ -38,6 +39,34 @@ var (
3839
tl TL
3940
)
4041

42+
// Issue 9370
43+
func TestCmpIfaceConcreteAlloc(t *testing.T) {
44+
if runtime.Compiler != "gc" {
45+
t.Skip("skipping on non-gc compiler")
46+
}
47+
48+
n := testing.AllocsPerRun(1, func() {
49+
_ = e == ts
50+
_ = i1 == ts
51+
})
52+
53+
if n > 0 {
54+
t.Fatalf("iface cmp allocs=%v; want 0", n)
55+
}
56+
}
57+
58+
func BenchmarkCmpEfaceConcrete(b *testing.B) {
59+
for i := 0; i < b.N; i++ {
60+
_ = e == ts
61+
}
62+
}
63+
64+
func BenchmarkCmpIfaceConcrete(b *testing.B) {
65+
for i := 0; i < b.N; i++ {
66+
_ = i1 == ts
67+
}
68+
}
69+
4170
func BenchmarkConvT2ESmall(b *testing.B) {
4271
for i := 0; i < b.N; i++ {
4372
e = ts

test/fixedbugs/issue9370.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// errorcheck
2+
3+
// Copyright 2009 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+
// Verify that concrete/interface comparisons are
8+
// typechecked correctly by the compiler.
9+
10+
package main
11+
12+
type I interface {
13+
Method()
14+
}
15+
16+
type C int
17+
18+
func (C) Method() {}
19+
20+
var (
21+
e interface{}
22+
i I
23+
c C
24+
n int
25+
)
26+
27+
var (
28+
_ = e == c
29+
_ = e != c
30+
_ = e >= c // ERROR "invalid operation.*not defined"
31+
_ = c == e
32+
_ = c != e
33+
_ = c >= e // ERROR "invalid operation.*not defined"
34+
35+
_ = i == c
36+
_ = i != c
37+
_ = i >= c // ERROR "invalid operation.*not defined"
38+
_ = c == i
39+
_ = c != i
40+
_ = c >= i // ERROR "invalid operation.*not defined"
41+
42+
_ = e == n
43+
_ = e != n
44+
_ = e >= n // ERROR "invalid operation.*not defined"
45+
_ = n == e
46+
_ = n != e
47+
_ = n >= e // ERROR "invalid operation.*not defined"
48+
49+
// i and n are not assignable to each other
50+
_ = i == n // ERROR "invalid operation.*mismatched types"
51+
_ = i != n // ERROR "invalid operation.*mismatched types"
52+
_ = i >= n // ERROR "invalid operation.*mismatched types"
53+
_ = n == i // ERROR "invalid operation.*mismatched types"
54+
_ = n != i // ERROR "invalid operation.*mismatched types"
55+
_ = n >= i // ERROR "invalid operation.*mismatched types"
56+
)

test/live.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,7 @@ var i9 interface{}
137137
func f9() bool {
138138
g8()
139139
x := i9
140-
// using complex number in comparison so that
141-
// there is always a convT2E, no matter what the
142-
// interface rules are.
143-
return x != 99.0i // ERROR "live at call to convT2E: x"
140+
return x != 99.0i
144141
}
145142

146143
// liveness formerly confused by UNDEF followed by RET,

0 commit comments

Comments
 (0)