]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/gc: evaluate concrete == interface without allocating
authorJosh Bleecher Snyder <josharian@gmail.com>
Wed, 24 Dec 2014 02:28:02 +0000 (18:28 -0800)
committerJosh Bleecher Snyder <josharian@gmail.com>
Thu, 12 Feb 2015 22:23:38 +0000 (22:23 +0000)
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

Evaluating I(c) can allocate.

This CL changes the evaluation of i==c to
x, ok := i.(C); ok && x == c

The new generated code is shorter and does not allocate directly.

If C is small, as it is in every instance in the stdlib,
the new code also uses less stack space
and makes one runtime call instead of two.

If C is very large, the original implementation is used.
The cutoff for "very large" is 1<<16,
following the stack vs heap cutoff used elsewhere.

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
BenchmarkEqEfaceConcrete      29.5          7.92          -73.15%
BenchmarkEqIfaceConcrete      32.1          7.90          -75.39%
BenchmarkNeEfaceConcrete      29.9          7.90          -73.58%
BenchmarkNeIfaceConcrete      35.9          7.90          -77.99%

Fixes #9370.

Change-Id: I7c4555950bcd6406ee5c613be1f2128da2c9a2b7
Reviewed-on: https://go-review.googlesource.com/2096
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>

src/cmd/gc/typecheck.c
src/cmd/gc/walk.c
src/runtime/iface_test.go
test/fixedbugs/issue9370.go [new file with mode: 0644]
test/live.go

index 635d2c4170a3f966a9d5af4893ba8ccc5fda0466..64b80a88cd10ca1586c7c13be5aaa8a6fddd6dc4 100644 (file)
@@ -570,6 +570,7 @@ reswitch:
                et = t->etype;
                if(et == TIDEAL)
                        et = TINT;
+               aop = 0;
                if(iscmp[n->op] && t->etype != TIDEAL && !eqtype(l->type, r->type)) {
                        // comparison is okay as long as one side is
                        // assignable to the other.  convert so they have
@@ -577,16 +578,20 @@ reswitch:
                        //
                        // the only conversion that isn't a no-op is concrete == interface.
                        // in that case, check comparability of the concrete type.
+                       // The conversion allocates, so only do it if the concrete type is huge.
                        if(r->type->etype != TBLANK && (aop = assignop(l->type, r->type, nil)) != 0) {
                                if(isinter(r->type) && !isinter(l->type) && algtype1(l->type, nil) == ANOEQ) {
                                        yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(l->type));
                                        goto error;
                                }
-                               l = nod(aop, l, N);
-                               l->type = r->type;
-                               l->typecheck = 1;
-                               n->left = l;
-                               t = l->type;
+                               dowidth(l->type);
+                               if(isinter(r->type) == isinter(l->type) || l->type->width >= 1<<16) {
+                                       l = nod(aop, l, N);
+                                       l->type = r->type;
+                                       l->typecheck = 1;
+                                       n->left = l;
+                               }
+                               t = r->type;
                                goto converted;
                        }
                        if(l->type->etype != TBLANK && (aop = assignop(r->type, l->type, nil)) != 0) {
@@ -594,11 +599,14 @@ reswitch:
                                        yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(r->type));
                                        goto error;
                                }
-                               r = nod(aop, r, N);
-                               r->type = l->type;
-                               r->typecheck = 1;
-                               n->right = r;
-                               t = r->type;
+                               dowidth(r->type);
+                               if(isinter(r->type) == isinter(l->type) || r->type->width >= 1<<16) {
+                                       r = nod(aop, r, N);
+                                       r->type = l->type;
+                                       r->typecheck = 1;
+                                       n->right = r;
+                               }
+                               t = l->type;
                        }
                converted:
                        et = t->etype;
@@ -609,8 +617,10 @@ reswitch:
                                yyerror("invalid operation: %N (non-numeric type %T)", n, l->type);
                                goto error;
                        }
-                       yyerror("invalid operation: %N (mismatched types %T and %T)", n, l->type, r->type);
-                       goto error;
+                       if(isinter(r->type) == isinter(l->type) || aop == 0) {
+                               yyerror("invalid operation: %N (mismatched types %T and %T)", n, l->type, r->type);
+                               goto error;
+                       }
                }
                if(!okfor[op][et]) {
                        yyerror("invalid operation: %N (operator %O not defined on %s)", n, op, typekind(t));
@@ -685,7 +695,7 @@ reswitch:
                                n->right = l;
                        } else if(r->op == OLITERAL && r->val.ctype == CTNIL) {
                                // leave alone for back end
-                       } else {
+                       } else if(isinter(r->type) == isinter(l->type)) {
                                n->etype = n->op;
                                n->op = OCMPIFACE;
                        }
index e2d74e46bc991342941d59d155d8757343916e9f..aed5e33a60f3c19e8c480b793fcdc0513047ca74 100644 (file)
@@ -3339,10 +3339,51 @@ static void
 walkcompare(Node **np, NodeList **init)
 {
        Node *n, *l, *r, *call, *a, *li, *ri, *expr, *cmpl, *cmpr;
+       Node *x, *ok;
        int andor, i, needsize;
        Type *t, *t1;
        
        n = *np;
+
+       // Given interface value l and concrete value r, rewrite
+       //   l == r
+       // to
+       //   x, ok := l.(type(r)); ok && x == r
+       // Handle != similarly.
+       // This avoids the allocation that would be required
+       // to convert r to l for comparison.
+       l = N;
+       r = N;
+       if(isinter(n->left->type) && !isinter(n->right->type)) {
+               l = n->left;
+               r = n->right;
+       } else if(!isinter(n->left->type) && isinter(n->right->type)) {
+               l = n->right;
+               r = n->left;
+       }
+       if(l != N) {
+               x = temp(r->type);
+               ok = temp(types[TBOOL]);
+
+               // l.(type(r))
+               a = nod(ODOTTYPE, l, N);
+               a->type = r->type;
+
+               // x, ok := l.(type(r))
+               expr = nod(OAS2, N, N);
+               expr->list = list1(x);
+               expr->list = list(expr->list, ok);
+               expr->rlist = list1(a);
+               typecheck(&expr, Etop);
+               walkexpr(&expr, init);
+
+               if(n->op == OEQ)
+                       r = nod(OANDAND, ok, nod(OEQ, x, r));
+               else
+                       r = nod(OOROR, nod(ONOT, ok, N), nod(ONE, x, r));
+               *init = list(*init, expr);
+               goto ret;
+       }
        
        // Must be comparison of array or struct.
        // Otherwise back end handles it.
index bca0ea0ee758334082a13f3a4c2a942bda6b1297..bfeb94b8aa768898e4a22108f1e1c424df0e0c7f 100644 (file)
@@ -5,6 +5,7 @@
 package runtime_test
 
 import (
+       "runtime"
        "testing"
 )
 
@@ -38,6 +39,47 @@ var (
        tl TL
 )
 
+// Issue 9370
+func TestCmpIfaceConcreteAlloc(t *testing.T) {
+       if runtime.Compiler != "gc" {
+               t.Skip("skipping on non-gc compiler")
+       }
+
+       n := testing.AllocsPerRun(1, func() {
+               _ = e == ts
+               _ = i1 == ts
+               _ = e == 1
+       })
+
+       if n > 0 {
+               t.Fatalf("iface cmp allocs=%v; want 0", n)
+       }
+}
+
+func BenchmarkEqEfaceConcrete(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               _ = e == ts
+       }
+}
+
+func BenchmarkEqIfaceConcrete(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               _ = i1 == ts
+       }
+}
+
+func BenchmarkNeEfaceConcrete(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               _ = e != ts
+       }
+}
+
+func BenchmarkNeIfaceConcrete(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               _ = i1 != ts
+       }
+}
+
 func BenchmarkConvT2ESmall(b *testing.B) {
        for i := 0; i < b.N; i++ {
                e = ts
diff --git a/test/fixedbugs/issue9370.go b/test/fixedbugs/issue9370.go
new file mode 100644 (file)
index 0000000..120af35
--- /dev/null
@@ -0,0 +1,127 @@
+// errorcheck
+
+// Copyright 2014 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Verify that concrete/interface comparisons are
+// typechecked correctly by the compiler.
+
+package main
+
+type I interface {
+       Method()
+}
+
+type C int
+
+func (C) Method() {}
+
+type G func()
+
+func (G) Method() {}
+
+var (
+       e interface{}
+       i I
+       c C
+       n int
+       f func()
+       g G
+)
+
+var (
+       _ = e == c
+       _ = e != c
+       _ = e >= c // ERROR "invalid operation.*not defined"
+       _ = c == e
+       _ = c != e
+       _ = c >= e // ERROR "invalid operation.*not defined"
+
+       _ = i == c
+       _ = i != c
+       _ = i >= c // ERROR "invalid operation.*not defined"
+       _ = c == i
+       _ = c != i
+       _ = c >= i // ERROR "invalid operation.*not defined"
+
+       _ = e == n
+       _ = e != n
+       _ = e >= n // ERROR "invalid operation.*not defined"
+       _ = n == e
+       _ = n != e
+       _ = n >= e // ERROR "invalid operation.*not defined"
+
+       // i and n are not assignable to each other
+       _ = i == n // ERROR "invalid operation.*mismatched types"
+       _ = i != n // ERROR "invalid operation.*mismatched types"
+       _ = i >= n // ERROR "invalid operation.*mismatched types"
+       _ = n == i // ERROR "invalid operation.*mismatched types"
+       _ = n != i // ERROR "invalid operation.*mismatched types"
+       _ = n >= i // ERROR "invalid operation.*mismatched types"
+
+       _ = e == 1
+       _ = e != 1
+       _ = e >= 1 // ERROR "invalid operation.*not defined"
+       _ = 1 == e
+       _ = 1 != e
+       _ = 1 >= e // ERROR "invalid operation.*not defined"
+
+       _ = i == 1 // ERROR "invalid operation.*mismatched types"
+       _ = i != 1 // ERROR "invalid operation.*mismatched types"
+       _ = i >= 1 // ERROR "invalid operation.*mismatched types"
+       _ = 1 == i // ERROR "invalid operation.*mismatched types"
+       _ = 1 != i // ERROR "invalid operation.*mismatched types"
+       _ = 1 >= i // ERROR "invalid operation.*mismatched types"
+
+       _ = e == f // ERROR "invalid operation.*not defined"
+       _ = e != f // ERROR "invalid operation.*not defined"
+       _ = e >= f // ERROR "invalid operation.*not defined"
+       _ = f == e // ERROR "invalid operation.*not defined"
+       _ = f != e // ERROR "invalid operation.*not defined"
+       _ = f >= e // ERROR "invalid operation.*not defined"
+
+       _ = i == f // ERROR "invalid operation.*mismatched types"
+       _ = i != f // ERROR "invalid operation.*mismatched types"
+       _ = i >= f // ERROR "invalid operation.*mismatched types"
+       _ = f == i // ERROR "invalid operation.*mismatched types"
+       _ = f != i // ERROR "invalid operation.*mismatched types"
+       _ = f >= i // ERROR "invalid operation.*mismatched types"
+
+       _ = e == g // ERROR "invalid operation.*not defined"
+       _ = e != g // ERROR "invalid operation.*not defined"
+       _ = e >= g // ERROR "invalid operation.*not defined"
+       _ = g == e // ERROR "invalid operation.*not defined"
+       _ = g != e // ERROR "invalid operation.*not defined"
+       _ = g >= e // ERROR "invalid operation.*not defined"
+
+       _ = i == g // ERROR "invalid operation.*not defined"
+       _ = i != g // ERROR "invalid operation.*not defined"
+       _ = i >= g // ERROR "invalid operation.*not defined"
+       _ = g == i // ERROR "invalid operation.*not defined"
+       _ = g != i // ERROR "invalid operation.*not defined"
+       _ = g >= i // ERROR "invalid operation.*not defined"
+
+       _ = _ == e // ERROR "cannot use _ as value"
+       _ = _ == i // ERROR "cannot use _ as value"
+       _ = _ == c // ERROR "cannot use _ as value"
+       _ = _ == n // ERROR "cannot use _ as value"
+       _ = _ == f // ERROR "cannot use _ as value"
+       _ = _ == g // ERROR "cannot use _ as value"
+
+       _ = e == _ // ERROR "cannot use _ as value"
+       _ = i == _ // ERROR "cannot use _ as value"
+       _ = c == _ // ERROR "cannot use _ as value"
+       _ = n == _ // ERROR "cannot use _ as value"
+       _ = f == _ // ERROR "cannot use _ as value"
+       _ = g == _ // ERROR "cannot use _ as value"
+
+       _ = _ == _ // ERROR "cannot use _ as value"
+
+       _ = e ^ c // ERROR "invalid operation.*mismatched types"
+       _ = c ^ e // ERROR "invalid operation.*mismatched types"
+       _ = 1 ^ e // ERROR "invalid operation.*mismatched types"
+       _ = e ^ 1 // ERROR "invalid operation.*mismatched types"
+       _ = 1 ^ c
+       _ = c ^ 1
+)
index f96bbcc6c0d6dd328b9eea26c111071aace79366..2f421066a5f22b1e0946a8f74d17fa83e9128faf 100644 (file)
@@ -137,10 +137,7 @@ var i9 interface{}
 func f9() bool {
        g8()
        x := i9
-       // using complex number in comparison so that
-       // there is always a convT2E, no matter what the
-       // interface rules are.
-       return x != 99.0i // ERROR "live at call to convT2E: x"
+       return x != interface{}(99.0i) // ERROR "live at call to convT2E: x"
 }
 
 // liveness formerly confused by UNDEF followed by RET,