From: Russ Cox Date: Thu, 16 Apr 2015 20:22:30 +0000 (-0400) Subject: cmd/internal/gc: clean up componentgen X-Git-Tag: go1.5beta1~1008 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=35b1dcc25f88dc54039dfa9b886f7940b4ae7a8f;p=gostls13.git cmd/internal/gc: clean up componentgen This is primarily about making the code clearer, but as part of the cleanup componentgen is now much more consistent about what it does and does not attempt. The new limit is to 8 move instructions. The old limit was either 3 or 4 small things but in the details it was quite inconsistent: ints, interfaces, strings, and slices all counted as small; it handled a struct containing two ints, but not a struct containing a struct containing two ints; it handled slices and interfaces and a struct containing a slice but not a struct containing an interface; and so on. The new code runs at about the same speed as the old code if limited to 4 moves, but that's much more restrictive when the pieces are strings or interfaces. With the limit raised to 8 moves, this CL is sometimes a significant improvement: benchmark old ns/op new ns/op delta BenchmarkBinaryTree17 4361174290 4362870005 +0.04% BenchmarkFannkuch11 3008201483 2974408533 -1.12% BenchmarkFmtFprintfEmpty 79.0 79.5 +0.63% BenchmarkFmtFprintfString 281 261 -7.12% BenchmarkFmtFprintfInt 264 262 -0.76% BenchmarkFmtFprintfIntInt 447 443 -0.89% BenchmarkFmtFprintfPrefixedInt 354 361 +1.98% BenchmarkFmtFprintfFloat 500 452 -9.60% BenchmarkFmtManyArgs 1688 1693 +0.30% BenchmarkGobDecode 11718456 11741179 +0.19% BenchmarkGobEncode 10144620 10161627 +0.17% BenchmarkGzip 437631642 435271877 -0.54% BenchmarkGunzip 109468858 110173606 +0.64% BenchmarkHTTPClientServer 76248 75362 -1.16% BenchmarkJSONEncode 24160474 23753091 -1.69% BenchmarkJSONDecode 84470041 82902026 -1.86% BenchmarkMandelbrot200 4676857 4687040 +0.22% BenchmarkGoParse 4954602 4923965 -0.62% BenchmarkRegexpMatchEasy0_32 151 151 +0.00% BenchmarkRegexpMatchEasy0_1K 450 452 +0.44% BenchmarkRegexpMatchEasy1_32 131 130 -0.76% BenchmarkRegexpMatchEasy1_1K 713 695 -2.52% BenchmarkRegexpMatchMedium_32 227 218 -3.96% BenchmarkRegexpMatchMedium_1K 63911 62966 -1.48% BenchmarkRegexpMatchHard_32 3163 3026 -4.33% BenchmarkRegexpMatchHard_1K 93985 90266 -3.96% BenchmarkRevcomp 650697093 649211600 -0.23% BenchmarkTemplate 107049170 106804076 -0.23% BenchmarkTimeParse 448 452 +0.89% BenchmarkTimeFormat 468 460 -1.71% Change-Id: I08563133883e88bb9db9e9e4dee438a5af2787da Reviewed-on: https://go-review.googlesource.com/9004 Reviewed-by: Josh Bleecher Snyder --- diff --git a/src/cmd/internal/gc/gen.go b/src/cmd/internal/gc/gen.go index 9de41910f6..620ef3bc5d 100644 --- a/src/cmd/internal/gc/gen.go +++ b/src/cmd/internal/gc/gen.go @@ -1129,52 +1129,19 @@ func checklabels() { // elements of basic type are also supported. // nr is nil when assigning a zero value. func Componentgen(nr *Node, nl *Node) bool { - var nodl, nodr Node - - switch nl.Type.Etype { - default: + // Count number of moves required to move components. + const maxMoves = 8 + n := 0 + visitComponents(nl.Type, 0, func(t *Type, offset int64) bool { + n++ + return n <= maxMoves + }) + if n > maxMoves { return false - - case TARRAY: - t := nl.Type - - // Slices are ok. - if Isslice(t) { - break - } - - // Small arrays are ok. - if t.Bound > 0 && t.Bound <= 3 && !Isfat(t.Type) { - break - } - - return false - - case TSTRUCT: - // Small structs with non-fat types are ok. - // Zero-sized structs are treated separately elsewhere. - fldcount := int64(0) - - for t := nl.Type.Type; t != nil; t = t.Down { - if Isfat(t.Type) && !Isslice(t) { - return false - } - if t.Etype != TFIELD { - Fatal("componentgen: not a TFIELD: %v", Tconv(t, obj.FmtLong)) - } - fldcount++ - } - - if fldcount == 0 || fldcount > 4 { - return false - } - - case TSTRING, TINTER: - break } isConstString := Isconst(nr, CTSTR) - nodl = *nl + nodl := *nl if !cadable(nl) { if nr != nil && !cadable(nr) && !isConstString { return false @@ -1182,195 +1149,195 @@ func Componentgen(nr *Node, nl *Node) bool { Igen(nl, &nodl, nil) defer Regfree(&nodl) } + lbase := nodl.Xoffset - if nr != nil { - nodr = *nr - if !cadable(nr) && !isConstString { - Igen(nr, &nodr, nil) - defer Regfree(&nodr) + // Must call emitVardef on every path out of this function, + // but only after evaluating rhs. + emitVardef := func() { + // Emit vardef if needed. + if nl.Op == ONAME { + switch nl.Type.Etype { + case TARRAY, TSTRING, TINTER, TSTRUCT: + Gvardef(nl) + } } - } else { + } + + // Special case: zeroing. + var nodr Node + if nr == nil { // When zeroing, prepare a register containing zero. + // TODO(rsc): Check that this is actually generating the best code. if Thearch.REGZERO != 0 { // cpu has a dedicated zero register Nodreg(&nodr, Types[TUINT], Thearch.REGZERO) } else { // no dedicated zero register - var tmp Node - Nodconst(&tmp, nl.Type, 0) - + var zero Node + Nodconst(&zero, nl.Type, 0) Regalloc(&nodr, Types[TUINT], nil) - Thearch.Gmove(&tmp, &nodr) + Thearch.Gmove(&zero, &nodr) defer Regfree(&nodr) } - } - - // nl and nr are 'cadable' which basically means they are names (variables) now. - // If they are the same variable, don't generate any code, because the - // VARDEF we generate will mark the old value as dead incorrectly. - // (And also the assignments are useless.) - if nr != nil && nl.Op == ONAME && nr.Op == ONAME && nl == nr { - return true - } - switch nl.Type.Etype { - default: - return false - - case TARRAY: - // componentgen for arrays. - if nl.Op == ONAME { - Gvardef(nl) - } - t := nl.Type - if !Isslice(t) { - nodl.Type = t.Type - nodr.Type = nodl.Type - for fldcount := int64(0); fldcount < t.Bound; fldcount++ { - if nr == nil { - Clearslim(&nodl) - } else { - Thearch.Gmove(&nodr, &nodl) - } - nodl.Xoffset += t.Type.Width - nodr.Xoffset += t.Type.Width + emitVardef() + visitComponents(nl.Type, 0, func(t *Type, offset int64) bool { + nodl.Type = t + nodl.Xoffset = lbase + offset + nodr.Type = t + if Isfloat[t.Etype] { + // TODO(rsc): Cache zero register like we do for integers? + Clearslim(&nodl) + } else { + Thearch.Gmove(&nodr, &nodl) } return true - } - - // componentgen for slices. - nodl.Xoffset += int64(Array_array) + }) + return true + } - nodl.Type = Ptrto(nl.Type.Type) - - if nr != nil { - nodr.Xoffset += int64(Array_array) - nodr.Type = nodl.Type - } + // Special case: assignment of string constant. + if isConstString { + emitVardef() + // base + nodl.Type = Ptrto(Types[TUINT8]) + Regalloc(&nodr, Types[Tptr], nil) + p := Thearch.Gins(Thearch.Optoas(OAS, Types[Tptr]), nil, &nodr) + Datastring(nr.Val.U.Sval, &p.From) + p.From.Type = obj.TYPE_ADDR Thearch.Gmove(&nodr, &nodl) + Regfree(&nodr) - nodl.Xoffset += int64(Array_nel) - int64(Array_array) + // length nodl.Type = Types[Simtype[TUINT]] - - if nr != nil { - nodr.Xoffset += int64(Array_nel) - int64(Array_array) - nodr.Type = nodl.Type - } - + nodl.Xoffset += int64(Array_nel) - int64(Array_array) + Nodconst(&nodr, nodl.Type, int64(len(nr.Val.U.Sval))) Thearch.Gmove(&nodr, &nodl) + return true + } - nodl.Xoffset += int64(Array_cap) - int64(Array_nel) - nodl.Type = Types[Simtype[TUINT]] - - if nr != nil { - nodr.Xoffset += int64(Array_cap) - int64(Array_nel) - nodr.Type = nodl.Type - } + // General case: copy nl = nr. + nodr = *nr + if !cadable(nr) { + Igen(nr, &nodr, nil) + defer Regfree(&nodr) + } + rbase := nodr.Xoffset + + // Don't generate any code for complete copy of a variable into itself. + // It's useless, and the VARDEF will incorrectly mark the old value as dead. + // (This check assumes that the arguments passed to componentgen did not + // themselves come from Igen, or else we could have Op==ONAME but + // with a Type and Xoffset describing an individual field, not the entire + // variable.) + if nl.Op == ONAME && nr.Op == ONAME && nl == nr { + return true + } + emitVardef() + visitComponents(nl.Type, 0, func(t *Type, offset int64) bool { + nodl.Type = t + nodl.Xoffset = lbase + offset + nodr.Type = t + nodr.Xoffset = rbase + offset Thearch.Gmove(&nodr, &nodl) return true + }) + return true +} - case TSTRING: - if nl.Op == ONAME { - Gvardef(nl) +// visitComponents walks the individual components of the type t, +// walking into array elements, struct fields, the real and imaginary +// parts of complex numbers, and on 32-bit systems the high and +// low halves of 64-bit integers. +// It calls f for each such component, passing the component (aka element) +// type and memory offset, assuming t starts at startOffset. +// If f ever returns false, visitComponents returns false without any more +// calls to f. Otherwise visitComponents returns true. +func visitComponents(t *Type, startOffset int64, f func(elem *Type, elemOffset int64) bool) bool { + switch t.Etype { + case TINT64: + if Widthreg == 8 { + break } - nodl.Xoffset += int64(Array_array) - nodl.Type = Ptrto(Types[TUINT8]) - - if isConstString { - Regalloc(&nodr, Types[Tptr], nil) - p := Thearch.Gins(Thearch.Optoas(OAS, Types[Tptr]), nil, &nodr) - Datastring(nr.Val.U.Sval, &p.From) - p.From.Type = obj.TYPE_ADDR - Regfree(&nodr) - } else if nr != nil { - nodr.Xoffset += int64(Array_array) - nodr.Type = nodl.Type + // NOTE: Assuming little endian (signed top half at offset 4). + // We don't have any 32-bit big-endian systems. + if Thearch.Thechar != '5' && Thearch.Thechar != '8' { + Fatal("unknown 32-bit architecture") } + return f(Types[TUINT32], startOffset) && + f(Types[TINT32], startOffset+4) - Thearch.Gmove(&nodr, &nodl) + case TUINT64: + if Widthreg == 8 { + break + } + return f(Types[TUINT32], startOffset) && + f(Types[TUINT32], startOffset+4) - nodl.Xoffset += int64(Array_nel) - int64(Array_array) - nodl.Type = Types[Simtype[TUINT]] + case TCOMPLEX64: + return f(Types[TFLOAT32], startOffset) && + f(Types[TFLOAT32], startOffset+4) - if isConstString { - Nodconst(&nodr, nodl.Type, int64(len(nr.Val.U.Sval))) - } else if nr != nil { - nodr.Xoffset += int64(Array_nel) - int64(Array_array) - nodr.Type = nodl.Type - } + case TCOMPLEX128: + return f(Types[TFLOAT64], startOffset) && + f(Types[TFLOAT64], startOffset+8) - Thearch.Gmove(&nodr, &nodl) + case TINTER: + return f(Ptrto(Types[TUINT8]), startOffset) && + f(Ptrto(Types[TUINT8]), startOffset+int64(Widthptr)) return true - case TINTER: - if nl.Op == ONAME { - Gvardef(nl) - } - nodl.Xoffset += int64(Array_array) - nodl.Type = Ptrto(Types[TUINT8]) + case TSTRING: + return f(Ptrto(Types[TUINT8]), startOffset) && + f(Types[Simtype[TUINT]], startOffset+int64(Widthptr)) - if nr != nil { - nodr.Xoffset += int64(Array_array) - nodr.Type = nodl.Type + case TARRAY: + if Isslice(t) { + return f(Ptrto(t.Type), startOffset+int64(Array_array)) && + f(Types[Simtype[TUINT]], startOffset+int64(Array_nel)) && + f(Types[Simtype[TUINT]], startOffset+int64(Array_cap)) } - Thearch.Gmove(&nodr, &nodl) - - nodl.Xoffset += int64(Array_nel) - int64(Array_array) - nodl.Type = Ptrto(Types[TUINT8]) - - if nr != nil { - nodr.Xoffset += int64(Array_nel) - int64(Array_array) - nodr.Type = nodl.Type + // Short-circuit [1e6]struct{}. + if t.Type.Width == 0 { + return true } - Thearch.Gmove(&nodr, &nodl) + for i := int64(0); i < t.Bound; i++ { + if !visitComponents(t.Type, startOffset+i*t.Type.Width, f) { + return false + } + } return true case TSTRUCT: - if nl.Op == ONAME { - Gvardef(nl) - } - loffset := nodl.Xoffset - roffset := nodr.Xoffset - - // funarg structs may not begin at offset zero. - if nl.Type.Etype == TSTRUCT && nl.Type.Funarg != 0 && nl.Type.Type != nil { - loffset -= nl.Type.Type.Width - } - if nr != nil && nr.Type.Etype == TSTRUCT && nr.Type.Funarg != 0 && nr.Type.Type != nil { - roffset -= nr.Type.Type.Width - } - - for t := nl.Type.Type; t != nil; t = t.Down { - nodl.Xoffset = loffset + t.Width - nodl.Type = t.Type - - if nr == nil { - Clearslim(&nodl) - } else { - nodr.Xoffset = roffset + t.Width - nodr.Type = nodl.Type - Thearch.Gmove(&nodr, &nodl) + if t.Type != nil && t.Type.Width != 0 { + // NOTE(rsc): If this happens, the right thing to do is to say + // startOffset -= t.Type.Width + // but I want to see if it does. + // The old version of componentgen handled this, + // in code introduced in CL 6932045 to fix issue #4518. + // But the test case in issue 4518 does not trigger this anymore, + // so maybe this complication is no longer needed. + Fatal("struct not at offset 0") + } + + for field := t.Type; field != nil; field = field.Down { + if field.Etype != TFIELD { + Fatal("bad struct") + } + if !visitComponents(field.Type, startOffset+field.Width, f) { + return false } } return true } + return f(t, startOffset) } func cadable(n *Node) bool { - if !n.Addable { - // dont know how it happens, - // but it does - return false - } - - switch n.Op { - case ONAME: - return true - } - - return false + // Note: Not sure why you can have n.Op == ONAME without n.Addable, but you can. + return n.Addable && n.Op == ONAME } diff --git a/src/cmd/internal/gc/plive.go b/src/cmd/internal/gc/plive.go index 7f19c75dc6..fe6905a062 100644 --- a/src/cmd/internal/gc/plive.go +++ b/src/cmd/internal/gc/plive.go @@ -235,6 +235,11 @@ func getvariables(fn *Node) []*Node { // is the index in the variables list. ll.N.Opt = nil + // The compiler doesn't emit initializations for zero-width parameters or results. + if ll.N.Type.Width == 0 { + continue + } + ll.N.Curfn = Curfn switch ll.N.Class { case PAUTO: