From: Josh Bleecher Snyder Date: Thu, 17 Mar 2016 19:24:11 +0000 (-0700) Subject: cmd/compile: make only one new Node in defaultlit X-Git-Tag: go1.7beta1~1034 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f32161daf8b14f49acc4a8e90262817e675c8309;p=gostls13.git 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 Reviewed-by: Matthew Dempsky --- diff --git a/src/cmd/compile/internal/gc/const.go b/src/cmd/compile/internal/gc/const.go index 6555cd80d0..17c67aa551 100644 --- 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 705ca5544b..e3690b2ae6 100644 --- 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