cmd/compile: make only one new Node in defaultlit

defaultlit and friends sometimes create a new
OLITERAL node, only to have replace it.
Thread hints when that is unnecessary.

name       old time/op     new time/op     delta
Template       318ms ± 6%      322ms ± 4%     ~           (p=0.154 n=24+25)
Unicode        162ms ± 6%      151ms ± 7%   -6.94%        (p=0.000 n=22+23)
GoTypes        1.04s ± 1%      1.04s ± 3%     ~           (p=0.136 n=20+25)
Compiler       5.08s ± 2%      5.10s ± 4%     ~           (p=0.788 n=25+25)
MakeBash       41.4s ± 1%      41.5s ± 1%     ~           (p=0.084 n=25+25)

name       old user-ns/op  new user-ns/op  delta
Template        438M ±10%       441M ± 9%     ~           (p=0.418 n=25+25)
Unicode         272M ± 5%       219M ± 5%  -19.33%        (p=0.000 n=24+21)
GoTypes        1.51G ± 3%      1.51G ± 3%     ~           (p=0.500 n=25+25)
Compiler       7.31G ± 3%      7.32G ± 3%     ~           (p=0.572 n=25+24)

name       old alloc/op    new alloc/op    delta
Template      57.3MB ± 0%     57.2MB ± 0%   -0.16%        (p=0.000 n=25+25)
Unicode       41.1MB ± 0%     38.7MB ± 0%   -5.81%        (p=0.000 n=25+25)
GoTypes        191MB ± 0%      191MB ± 0%   -0.06%        (p=0.000 n=25+25)
Compiler       840MB ± 0%      839MB ± 0%   -0.12%        (p=0.000 n=25+25)

name       old allocs/op   new allocs/op   delta
Template        500k ± 0%       500k ± 0%   -0.12%        (p=0.000 n=24+25)
Unicode         400k ± 0%       384k ± 0%   -4.16%        (p=0.000 n=25+25)
GoTypes        1.50M ± 0%      1.49M ± 0%   -0.05%        (p=0.000 n=25+25)
Compiler       6.04M ± 0%      6.03M ± 0%   -0.11%        (p=0.000 n=25+25)

Change-Id: I2fda5e072db67ba239848bde827c7deb2ad4abae
Reviewed-on: https://go-review.googlesource.com/20813
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/const.go b/src/cmd/compile/internal/gc/const.go
index 6555cd80d0dd6722f32f8f9b97790bcd13cf2774..17c67aa551c18f0b32f007dfffb6437327462980 100644 (file)
--- a/src/cmd/compile/internal/gc/const.go
+++ b/src/cmd/compile/internal/gc/const.go
@@ -95,20 +95,28 @@ func NegOne(t *Type) *Node {
        return n
 }
 
+// canReuseNode indicates whether it is known to be safe
+// to reuse a Node.
+type canReuseNode bool
+
+const (
+       noReuse canReuseNode = false // not necessarily safe to reuse
+       reuseOK canReuseNode = true  // safe to reuse
+)
+
 // convert n, if literal, to type t.
 // implicit conversion.
 // The result of convlit MUST be assigned back to n, e.g.
 //     n.Left = convlit(n.Left, t)
 func convlit(n *Node, t *Type) *Node {
-       return convlit1(n, t, false)
+       return convlit1(n, t, false, noReuse)
 }
 
-// convert n, if literal, to type t.
-// return a new node if necessary
-// (if n is a named constant, can't edit n->type directly).
+// convlit1 converts n, if literal, to type t.
+// It returns a new node if necessary.
 // The result of convlit1 MUST be assigned back to n, e.g.
-//     n.Left = convlit1(n.Left, t, explicit)
-func convlit1(n *Node, t *Type, explicit bool) *Node {
+//     n.Left = convlit1(n.Left, t, explicit, reuse)
+func convlit1(n *Node, t *Type, explicit bool, reuse canReuseNode) *Node {
        if n == nil || t == nil || n.Type == nil || isideal(t) || n.Type == t {
                return n
        }
@@ -116,9 +124,12 @@ func convlit1(n *Node, t *Type, explicit bool) *Node {
                return n
        }
 
-       if n.Op == OLITERAL {
+       if n.Op == OLITERAL && !reuse {
+               // Can't always set n.Type directly on OLITERAL nodes.
+               // See discussion on CL 20813.
                nn := *n
                n = &nn
+               reuse = true
        }
 
        switch n.Op {
@@ -142,11 +153,11 @@ func convlit1(n *Node, t *Type, explicit bool) *Node {
                // target is invalid type for a constant?  leave alone.
        case OLITERAL:
                if !okforconst[t.Etype] && n.Type.Etype != TNIL {
-                       return defaultlit(n, nil)
+                       return defaultlitreuse(n, nil, reuse)
                }
 
        case OLSH, ORSH:
-               n.Left = convlit1(n.Left, t, explicit && isideal(n.Left.Type))
+               n.Left = convlit1(n.Left, t, explicit && isideal(n.Left.Type), noReuse)
                t = n.Left.Type
                if t != nil && t.Etype == TIDEAL && n.Val().Ctype() != CTINT {
                        n.SetVal(toint(n.Val()))
@@ -202,7 +213,7 @@ func convlit1(n *Node, t *Type, explicit bool) *Node {
                        n.Type = t
                        return n
                }
-               return defaultlit(n, nil)
+               return defaultlitreuse(n, nil, reuse)
        }
 
        switch ct {
@@ -309,7 +320,7 @@ bad:
        }
 
        if isideal(n.Type) {
-               n = defaultlit(n, nil)
+               n = defaultlitreuse(n, nil, reuse)
        }
        return n
 }
@@ -663,8 +674,7 @@ func evconst(n *Node) {
                        OCONV_ | CTFLT_,
                        OCONV_ | CTSTR_,
                        OCONV_ | CTBOOL_:
-                       nl = convlit1(nl, n.Type, true)
-
+                       nl = convlit1(nl, n.Type, true, false)
                        v = nl.Val()
 
                case OPLUS_ | CTINT_,
@@ -1243,13 +1253,20 @@ func idealkind(n *Node) Ctype {
 // The result of defaultlit MUST be assigned back to n, e.g.
 //     n.Left = defaultlit(n.Left, t)
 func defaultlit(n *Node, t *Type) *Node {
+       return defaultlitreuse(n, t, noReuse)
+}
+
+// The result of defaultlitreuse MUST be assigned back to n, e.g.
+//     n.Left = defaultlitreuse(n.Left, t, reuse)
+func defaultlitreuse(n *Node, t *Type, reuse canReuseNode) *Node {
        if n == nil || !isideal(n.Type) {
                return n
        }
 
-       if n.Op == OLITERAL {
+       if n.Op == OLITERAL && !reuse {
                nn := *n
                n = &nn
+               reuse = true
        }
 
        lno := setlineno(n)
@@ -1274,7 +1291,7 @@ func defaultlit(n *Node, t *Type) *Node {
 
                if n.Val().Ctype() == CTSTR {
                        t1 := Types[TSTRING]
-                       n = convlit(n, t1)
+                       n = convlit1(n, t1, false, reuse)
                        break
                }
 
@@ -1288,7 +1305,7 @@ func defaultlit(n *Node, t *Type) *Node {
                if t != nil && t.Etype == TBOOL {
                        t1 = t
                }
-               n = convlit(n, t1)
+               n = convlit1(n, t1, false, reuse)
 
        case CTINT:
                t1 = Types[TINT]
@@ -1333,7 +1350,7 @@ num:
        if n.Val().Ctype() != CTxxx {
                overflow(n.Val(), t1)
        }
-       n = convlit(n, t1)
+       n = convlit1(n, t1, false, reuse)
        lineno = lno
        return n
 }

diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go
index 705ca5544b00e2a1cdf7362fe27c5a580579edf4..e3690b2ae6545bcfe9c9e82515dc01a64071b7cf 100644 (file)
--- a/src/cmd/compile/internal/gc/typecheck.go
+++ b/src/cmd/compile/internal/gc/typecheck.go
@@ -1722,7 +1722,7 @@ OpSwitch:
                ok |= Erv
                saveorignode(n)
                n.Left = typecheck(n.Left, Erv|top&(Eindir|Eiota))
-               n.Left = convlit1(n.Left, n.Type, true)
+               n.Left = convlit1(n.Left, n.Type, true, noReuse)
                t := n.Left.Type
                if t == nil || n.Type == nil {
                        n.Type = nil