The fconv flag arguments were 0, FmtSharp, and FmtSharp|FmtSign.
The 0 value was used for binary representation only, which was
readily available via Mpflt.String. Otherwise, FmtSharp was always
passed. FmtSign was used to print the '+' sign in case of a positive
number and only needed for complex number formatting. Instead
implemented cconv and handled it there.
Change-Id: I1f77282f995be9cfda05efb71a0e027836a9da26
Reviewed-on: https://go-review.googlesource.com/136195
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This change adds a new column, AST IR. That column contains
nodes for a function specified in $GOSSAFUNC.
Also this CL enables horizontal scrolling of sources and AST columns.
Fixes#26662
Change-Id: I3fba39fd998bb05e9c93038e8ec2384c69613b24
Reviewed-on: https://go-review.googlesource.com/126858
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Found while tracking down another issue.
Change-Id: If06eee72ce3e3f45dc00b3b68670e1e4d99c87a7
Reviewed-on: https://go-review.googlesource.com/116455
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, range loops over slices and arrays are compiled roughly
like:
for i, x := range s { b }
⇓
for i, _n, _p := 0, len(s), &s[0]; i < _n; i, _p = i+1, _p + unsafe.Sizeof(s[0]) { b }
⇓
i, _n, _p := 0, len(s), &s[0]
goto cond
body:
{ b }
i, _p = i+1, _p + unsafe.Sizeof(s[0])
cond:
if i < _n { goto body } else { goto end }
end:
The problem with this lowering is that _p may temporarily point past
the end of the allocation the moment before the loop terminates. Right
now this isn't a problem because there's never a safe-point during
this brief moment.
We're about to introduce safe-points everywhere, so this bad pointer
is going to be a problem. We could mark the increment as an unsafe
block, but this inhibits reordering opportunities and could result in
infrequent safe-points if the body is short.
Instead, this CL fixes this by changing how we compile range loops to
never produce this past-the-end pointer. It changes the lowering to
roughly:
i, _n, _p := 0, len(s), &s[0]
if i < _n { goto body } else { goto end }
top:
_p += unsafe.Sizeof(s[0])
body:
{ b }
i++
if i < _n { goto top } else { goto end }
end:
Notably, the increment is split into two parts: we increment the index
before checking the condition, but increment the pointer only *after*
the condition check has succeeded.
The implementation builds on the OFORUNTIL construct that was
introduced during the loop preemption experiments, since OFORUNTIL
places the increment and condition after the loop body. To support the
extra "late increment" step, we further define OFORUNTIL's "List"
field to contain the late increment statements. This makes all of this
a relatively small change.
This depends on the improvements to the prove pass in CL 102603. With
the current lowering, bounds-check elimination knows that i < _n in
the body because the body block is dominated by the cond block. In the
new lowering, deriving this fact requires detecting that i < _n on
*both* paths into body and hence is true in body. CL 102603 made prove
able to detect this.
The code size effect of this is minimal. The cmd/go binary on
linux/amd64 increases by 0.17%. Performance-wise, this actually
appears to be a net win, though it's mostly noise:
name old time/op new time/op delta
BinaryTree17-12 2.80s ± 0% 2.61s ± 1% -6.88% (p=0.000 n=20+18)
Fannkuch11-12 2.41s ± 0% 2.42s ± 0% +0.05% (p=0.005 n=20+20)
FmtFprintfEmpty-12 41.6ns ± 5% 41.4ns ± 6% ~ (p=0.765 n=20+19)
FmtFprintfString-12 69.4ns ± 3% 69.3ns ± 1% ~ (p=0.084 n=19+17)
FmtFprintfInt-12 76.1ns ± 1% 77.3ns ± 1% +1.57% (p=0.000 n=19+19)
FmtFprintfIntInt-12 122ns ± 2% 123ns ± 3% +0.95% (p=0.015 n=20+20)
FmtFprintfPrefixedInt-12 153ns ± 2% 151ns ± 3% -1.27% (p=0.013 n=20+20)
FmtFprintfFloat-12 215ns ± 0% 216ns ± 0% +0.47% (p=0.000 n=20+16)
FmtManyArgs-12 486ns ± 1% 498ns ± 0% +2.40% (p=0.000 n=20+17)
GobDecode-12 6.43ms ± 0% 6.50ms ± 0% +1.08% (p=0.000 n=18+19)
GobEncode-12 5.43ms ± 1% 5.47ms ± 0% +0.76% (p=0.000 n=20+20)
Gzip-12 218ms ± 1% 218ms ± 1% ~ (p=0.883 n=20+20)
Gunzip-12 38.8ms ± 0% 38.9ms ± 0% ~ (p=0.644 n=19+19)
HTTPClientServer-12 76.2µs ± 1% 76.4µs ± 2% ~ (p=0.218 n=20+20)
JSONEncode-12 12.2ms ± 0% 12.3ms ± 1% +0.45% (p=0.000 n=19+19)
JSONDecode-12 54.2ms ± 1% 53.3ms ± 0% -1.67% (p=0.000 n=20+20)
Mandelbrot200-12 3.71ms ± 0% 3.71ms ± 0% ~ (p=0.143 n=19+20)
GoParse-12 3.22ms ± 0% 3.19ms ± 1% -0.72% (p=0.000 n=20+20)
RegexpMatchEasy0_32-12 76.7ns ± 1% 75.8ns ± 1% -1.19% (p=0.000 n=20+17)
RegexpMatchEasy0_1K-12 245ns ± 1% 243ns ± 0% -0.72% (p=0.000 n=18+17)
RegexpMatchEasy1_32-12 71.9ns ± 0% 71.7ns ± 1% -0.39% (p=0.006 n=12+18)
RegexpMatchEasy1_1K-12 358ns ± 1% 354ns ± 1% -1.13% (p=0.000 n=20+19)
RegexpMatchMedium_32-12 105ns ± 2% 105ns ± 1% -0.63% (p=0.007 n=19+20)
RegexpMatchMedium_1K-12 31.9µs ± 1% 31.9µs ± 1% ~ (p=1.000 n=17+17)
RegexpMatchHard_32-12 1.51µs ± 1% 1.52µs ± 2% +0.46% (p=0.042 n=18+18)
RegexpMatchHard_1K-12 45.3µs ± 1% 45.5µs ± 2% +0.44% (p=0.029 n=18+19)
Revcomp-12 388ms ± 1% 385ms ± 0% -0.57% (p=0.000 n=19+18)
Template-12 63.0ms ± 1% 63.3ms ± 0% +0.50% (p=0.000 n=19+20)
TimeParse-12 309ns ± 1% 307ns ± 0% -0.62% (p=0.000 n=20+20)
TimeFormat-12 328ns ± 0% 333ns ± 0% +1.35% (p=0.000 n=19+19)
[Geo mean] 47.0µs 46.9µs -0.20%
(https://perf.golang.org/search?q=upload:20180326.1)
For #10958.
For #24543.
Change-Id: Icbd52e711fdbe7938a1fea3e6baca1104b53ac3a
Reviewed-on: https://go-review.googlesource.com/102604
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This reduces the API surface of Type slightly (for #25056), but also
makes it more consistent with the reflect and go/types APIs.
Passes toolstash-check.
Change-Id: Ief9a8eb461ae6e88895f347e2a1b7b8a62423222
Reviewed-on: https://go-review.googlesource.com/109138
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For struct fields and methods, Field.Nname was only used to store
position information, which means we're allocating an entire ONAME
Node+Name+Param structure just for one field. We can optimize away
these ONAME allocations by instead adding a Field.Pos field.
Unfortunately, we can't get rid of Field.Nname, because it's needed
for function parameters, so Field grows a little bit and now has more
redundant information in those cases. However, that was already the
case (e.g., Field.Sym and Field.Nname.Sym), and it's still a net win
for allocations as demonstrated by the benchmarks below.
Additionally, by moving the ONAME allocation for function parameters
to funcargs, we can avoid allocating them for function parameters that
aren't used in corresponding function bodies (e.g., interface methods,
function-typed variables, and imported functions/methods without
inline bodies).
name old time/op new time/op delta
Template 254ms ± 6% 251ms ± 6% -1.04% (p=0.000 n=487+488)
Unicode 128ms ± 7% 128ms ± 7% ~ (p=0.294 n=482+467)
GoTypes 862ms ± 5% 860ms ± 4% ~ (p=0.075 n=488+471)
Compiler 3.91s ± 4% 3.90s ± 4% -0.39% (p=0.000 n=468+473)
name old user-time/op new user-time/op delta
Template 339ms ±14% 336ms ±14% -1.02% (p=0.001 n=498+494)
Unicode 176ms ±18% 176ms ±25% ~ (p=0.940 n=491+499)
GoTypes 1.13s ± 8% 1.13s ± 9% ~ (p=0.157 n=496+493)
Compiler 5.24s ± 6% 5.21s ± 6% -0.57% (p=0.000 n=485+489)
name old alloc/op new alloc/op delta
Template 38.3MB ± 0% 37.3MB ± 0% -2.58% (p=0.000 n=499+497)
Unicode 29.1MB ± 0% 29.1MB ± 0% -0.03% (p=0.000 n=500+493)
GoTypes 116MB ± 0% 115MB ± 0% -0.65% (p=0.000 n=498+499)
Compiler 492MB ± 0% 487MB ± 0% -1.00% (p=0.000 n=497+498)
name old allocs/op new allocs/op delta
Template 364k ± 0% 360k ± 0% -1.15% (p=0.000 n=499+499)
Unicode 336k ± 0% 336k ± 0% -0.01% (p=0.000 n=500+493)
GoTypes 1.16M ± 0% 1.16M ± 0% -0.30% (p=0.000 n=499+499)
Compiler 4.54M ± 0% 4.51M ± 0% -0.58% (p=0.000 n=494+495)
Passes toolstash-check -gcflags=-dwarf=false. Changes DWARF output
because position information is now tracked more precisely for
function parameters.
Change-Id: Ib8077d70d564cc448c5e4290baceab3a4396d712
Reviewed-on: https://go-review.googlesource.com/108217
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Add helper methods that validate n.Op and convert to/from the
appropriate type.
Notably, there was a lot of code in walk.go that thought setting
Etype=1 on an OADDR node affected escape analysis.
Passes toolstash-check.
TBR=marvin
Change-Id: Ieae7c67225c1459c9719f9e6a748a25b975cf758
Reviewed-on: https://go-review.googlesource.com/99535
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
There were only two large classes of use for these variables:
1) Testing "funcdepth != 0" or "funcdepth > 0", which is equivalent to
checking "Curfn != nil".
2) In oldname, detecting whether a closure variable has been created
for the current function, which can be handled by instead testing
"n.Name.Curfn != Curfn".
Lastly, merge funcstart into funchdr, since it's only called once, and
it better matches up with funcbody now.
Passes toolstash-check.
Change-Id: I8fe159a9d37ef7debc4cd310354cea22a8b23394
Reviewed-on: https://go-review.googlesource.com/99076
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It's always useful to distinguish "bool" and "string" from "untyped
bool" and "untyped string", so change typefmt to do this
unconditionally.
Also, while here, replace a bare 0 with its named constant FErr.
Fixes#23833.
Change-Id: I3fcb8d7204686937439caaaf8b3973fc236d0387
Reviewed-on: https://go-review.googlesource.com/94021
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Since the slice of names is almost exactly the same as what stringer is
already generating for us.
Passes toolstash -cmp on std cmd.
Change-Id: I3f1e95efc690c0108236689e721627f00f79a461
Reviewed-on: https://go-review.googlesource.com/77190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
Now possible, since stringer just got the -trimprefix flag added.
While at it, simplify a few Op stringifications since we can now use %v,
and no longer have to worry about o<len(opnames).
Passes toolstash -cmp on std cmd.
Fixes#15462.
Change-Id: Icdcde0b0a5eb165d18488918175024da274f782b
Reviewed-on: https://go-review.googlesource.com/76790
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
Reduce the scope of some. Also remove vars that were simply the index or
the value in a range statement. While at it, remove a var that was
exactly the length of a slice.
Also replaced 'bad' with a more clear 'errored' of type bool, and
renamed a single-char name with a comment to a name that is
self-explanatory.
And removed a few unnecessary Index calls within loops.
Passes toolstash -cmp on std cmd.
Change-Id: I26eee5f04e8f7e5418e43e25dca34f89cca5c80a
Reviewed-on: https://go-review.googlesource.com/70930
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, we used OXFALL vs OFALL to distinguish fallthrough
statements that had been validated. Because in the Node AST we flatten
statement blocks, OXCASE and OXFALL needed to keep track of their
block scopes for this purpose.
Now that we have an AST that keeps these separate, we can just perform
the validation earlier.
Passes toolstash-check.
Fixes#14540.
Change-Id: I8421eaba16c2b3b72c9c5483b5cf20b14261385e
Reviewed-on: https://go-review.googlesource.com/61130
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
All of these are uints of different sizes, so checking >= 0 or < 0 are
effectively no-ops.
Found with staticcheck.
Change-Id: I16ac900eb7007bc8f9018b302136d42e483a4180
Reviewed-on: https://go-review.googlesource.com/56950
Reviewed-by: Matt Layher <mdlayher@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Matt Layher <mdlayher@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When package ssa was created, Type was in package gc.
To avoid circular dependencies, we used an interface (ssa.Type)
to represent type information in SSA.
In the Go 1.9 cycle, gri extricated the Type type from package gc.
As a result, we can now use it in package ssa.
Now, instead of package types depending on package ssa,
it is the other way.
This is a more sensible dependency tree,
and helps compiler performance a bit.
Though this is a big CL, most of the changes are
mechanical and uninteresting.
Interesting bits:
* Add new singleton globals to package types for the special
SSA types Memory, Void, Invalid, Flags, and Int128.
* Add two new Types, TSSA for the special types,
and TTUPLE, for SSA tuple types.
ssa.MakeTuple is now types.NewTuple.
* Move type comparison result constants CMPlt, CMPeq, and CMPgt
to package types.
* We had picked the name "types" in our rules for the handy
list of types provided by ssa.Config. That conflicted with
the types package name, so change it to "typ".
* Update the type comparison routine to handle tuples and special
types inline.
* Teach gc/fmt.go how to print special types.
* We can now eliminate ElemTypes in favor of just Elem,
and probably also some other duplicated Type methods
designed to return ssa.Type instead of *types.Type.
* The ssa tests were using their own dummy types,
and they were not particularly careful about types in general.
Of necessity, this CL switches them to use *types.Type;
it does not make them more type-accurate.
Unfortunately, using types.Type means initializing a bit
of the types universe.
This is prime for refactoring and improvement.
This shrinks ssa.Value; it now fits in a smaller size class
on 64 bit systems. This doesn't have a giant impact,
though, since most Values are preallocated in a chunk.
name old alloc/op new alloc/op delta
Template 37.9MB ± 0% 37.7MB ± 0% -0.57% (p=0.000 n=10+8)
Unicode 28.9MB ± 0% 28.7MB ± 0% -0.52% (p=0.000 n=10+10)
GoTypes 110MB ± 0% 109MB ± 0% -0.88% (p=0.000 n=10+10)
Flate 24.7MB ± 0% 24.6MB ± 0% -0.66% (p=0.000 n=10+10)
GoParser 31.1MB ± 0% 30.9MB ± 0% -0.61% (p=0.000 n=10+9)
Reflect 73.9MB ± 0% 73.4MB ± 0% -0.62% (p=0.000 n=10+8)
Tar 25.8MB ± 0% 25.6MB ± 0% -0.77% (p=0.000 n=9+10)
XML 41.2MB ± 0% 40.9MB ± 0% -0.80% (p=0.000 n=10+10)
[Geo mean] 40.5MB 40.3MB -0.68%
name old allocs/op new allocs/op delta
Template 385k ± 0% 386k ± 0% ~ (p=0.356 n=10+9)
Unicode 343k ± 1% 344k ± 0% ~ (p=0.481 n=10+10)
GoTypes 1.16M ± 0% 1.16M ± 0% -0.16% (p=0.004 n=10+10)
Flate 238k ± 1% 238k ± 1% ~ (p=0.853 n=10+10)
GoParser 320k ± 0% 320k ± 0% ~ (p=0.720 n=10+9)
Reflect 957k ± 0% 957k ± 0% ~ (p=0.460 n=10+8)
Tar 252k ± 0% 252k ± 0% ~ (p=0.133 n=9+10)
XML 400k ± 0% 400k ± 0% ~ (p=0.796 n=10+10)
[Geo mean] 428k 428k -0.01%
Removing all the interface calls helps non-trivially with CPU, though.
name old time/op new time/op delta
Template 178ms ± 4% 173ms ± 3% -2.90% (p=0.000 n=94+96)
Unicode 85.0ms ± 4% 83.9ms ± 4% -1.23% (p=0.000 n=96+96)
GoTypes 543ms ± 3% 528ms ± 3% -2.73% (p=0.000 n=98+96)
Flate 116ms ± 3% 113ms ± 4% -2.34% (p=0.000 n=96+99)
GoParser 144ms ± 3% 140ms ± 4% -2.80% (p=0.000 n=99+97)
Reflect 344ms ± 3% 334ms ± 4% -3.02% (p=0.000 n=100+99)
Tar 106ms ± 5% 103ms ± 4% -3.30% (p=0.000 n=98+94)
XML 198ms ± 5% 192ms ± 4% -2.88% (p=0.000 n=92+95)
[Geo mean] 178ms 173ms -2.65%
name old user-time/op new user-time/op delta
Template 229ms ± 5% 224ms ± 5% -2.36% (p=0.000 n=95+99)
Unicode 107ms ± 6% 106ms ± 5% -1.13% (p=0.001 n=93+95)
GoTypes 696ms ± 4% 679ms ± 4% -2.45% (p=0.000 n=97+99)
Flate 137ms ± 4% 134ms ± 5% -2.66% (p=0.000 n=99+96)
GoParser 176ms ± 5% 172ms ± 8% -2.27% (p=0.000 n=98+100)
Reflect 430ms ± 6% 411ms ± 5% -4.46% (p=0.000 n=100+92)
Tar 128ms ±13% 123ms ±13% -4.21% (p=0.000 n=100+100)
XML 239ms ± 6% 233ms ± 6% -2.50% (p=0.000 n=95+97)
[Geo mean] 220ms 213ms -2.76%
Change-Id: I15c7d6268347f8358e75066dfdbd77db24e8d0c1
Reviewed-on: https://go-review.googlesource.com/42145
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Compile:
package p
var f = func(...A)
Before this CL:
x.go:3:13: type %!v(PANIC=runtime error: invalid memory address or nil pointer dereference) is not an expression
x.go:3:17: undefined: A
After this CL:
x.go:3:13: type func(...<T>) is not an expression
x.go:3:17: undefined: A
Found with go-fuzz.
Fixes#20233
Change-Id: Ibb232b3954c4091071440eba48b44c4022a8083f
Reviewed-on: https://go-review.googlesource.com/42610
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When compiling the program:
package p
func _(){
*;:=
}
Before:
x.go:4:3: syntax error: unexpected semicolon, expecting expression
x.go:4:4: non-name *%!v(PANIC=runtime error: invalid memory address or nil pointer dereference) on left side of :=
x.go:5:1: syntax error: unexpected }, expecting expression
After:
x.go:4:3: syntax error: unexpected semicolon, expecting expression
x.go:4:4: non-name *<N> on left side of :=
x.go:5:1: syntax error: unexpected }, expecting expression
No test because:
(1) we don't have a good mechanism to check for the
absence of the string "PANIC" in an error message
(2) the string "*<N>", while better, is itself ugly enough
that I don't want to actively check for it
(3) the bug isn't very important, the kind of thing only fuzzers encounter
(4) the fix is obvious and trivial
Fixes#20220
Change-Id: I35faa986b60b671414ee999d6264b06937f250e3
Reviewed-on: https://go-review.googlesource.com/42498
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Node.Used was written to from the backend
concurrently with reads of Node.Class
for the same ONAME Nodes.
I do not know why it was not failing consistently
under the race detector, but it is a race.
This is likely also a problem with Node.HasVal and Node.HasOpt.
They will be handled in a separate CL.
Fix Used by moving it to gc.Name and making it a separate bool.
There was one non-Name use of Used, marking OLABELs as used.
That is no longer needed, now that goto and label checking
happens early in the front end.
Leave the getters and setters in place,
to ease changing the representation in the future
(or changing to an interface!).
Updates #20144
Change-Id: I9bec7c6d33dcb129a4cfa9d338462ea33087f9f7
Reviewed-on: https://go-review.googlesource.com/42015
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
- created new package cmd/compile/internal/types
- moved Pkg, Sym, Type to new package
- to break cycles, for now we need the (ugly) types/utils.go
file which contains a handful of functions that must be installed
early by the gc frontend
- to break cycles, for now we need two functions to convert between
*gc.Node and *types.Node (the latter is a dummy type)
- adjusted the gc's code to use the new package and the conversion
functions as needed
- made several Pkg, Sym, and Type methods functions as needed
- renamed constructors typ, typPtr, typArray, etc. to types.New,
types.NewPtr, types.NewArray, etc.
Passes toolstash-check -all.
Change-Id: I8adfa5e85c731645d0a7fd2030375ed6ebf54b72
Reviewed-on: https://go-review.googlesource.com/39855
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 38147 eliminated package gc globals in formatting routines.
However, tconv still used the Type field Trecur
to avoid infinite recursion when formatting recursive
interfaces types such as (test/fixedbugs398.go):
type i1 interface {
F() interface {
i1
}
}
type i2 interface {
F() interface {
i2
}
}
This CL changes the recursion prevention to use a parameter,
and threads it through the formatting routines.
Because this fundamentally limits the embedding depth
of all types, it sets the depth limit to be much higher.
In practice, it is unlikely to impact any code at all,
one way or the other.
The remaining uses of Type.Trecur are boolean in nature.
A future CL will change Type.Trecur to be a boolean flag.
The removal of a couple of mode.Sprintf calls
makes this a very minor net performance improvement:
name old alloc/op new alloc/op delta
Template 40.0MB ± 0% 40.0MB ± 0% -0.13% (p=0.032 n=5+5)
Unicode 30.0MB ± 0% 29.9MB ± 0% ~ (p=0.310 n=5+5)
GoTypes 114MB ± 0% 113MB ± 0% -0.25% (p=0.008 n=5+5)
SSA 856MB ± 0% 855MB ± 0% -0.04% (p=0.008 n=5+5)
Flate 25.5MB ± 0% 25.4MB ± 0% -0.27% (p=0.008 n=5+5)
GoParser 31.9MB ± 0% 31.9MB ± 0% ~ (p=0.222 n=5+5)
Reflect 79.0MB ± 0% 78.6MB ± 0% -0.45% (p=0.008 n=5+5)
Tar 26.8MB ± 0% 26.7MB ± 0% -0.25% (p=0.032 n=5+5)
XML 42.4MB ± 0% 42.4MB ± 0% ~ (p=0.151 n=5+5)
name old allocs/op new allocs/op delta
Template 395k ± 0% 391k ± 0% -1.00% (p=0.008 n=5+5)
Unicode 321k ± 1% 319k ± 0% -0.56% (p=0.008 n=5+5)
GoTypes 1.16M ± 0% 1.14M ± 0% -1.61% (p=0.008 n=5+5)
SSA 7.63M ± 0% 7.60M ± 0% -0.30% (p=0.008 n=5+5)
Flate 239k ± 0% 234k ± 0% -1.94% (p=0.008 n=5+5)
GoParser 320k ± 0% 317k ± 1% -0.86% (p=0.008 n=5+5)
Reflect 1.00M ± 0% 0.98M ± 0% -2.17% (p=0.016 n=4+5)
Tar 255k ± 1% 251k ± 0% -1.35% (p=0.008 n=5+5)
XML 398k ± 0% 395k ± 0% -0.89% (p=0.008 n=5+5)
Updates #15756
Change-Id: Id23e647d347aa841f9a69d51f7d2d7d27b259239
Reviewed-on: https://go-review.googlesource.com/38797
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Previously, we handled recursive interfaces by deferring typechecking
of interface methods, while eagerly expanding interface embeddings.
This CL switches to eagerly evaluating interface methods, and
deferring expanding interface embeddings to dowidth. This allows us to
detect recursive interface embeddings with the same mechanism used for
detecting recursive struct embeddings.
Updates #16369.
Change-Id: If4c0320058047f8a2d9b52b9a79de47eb9887f95
Reviewed-on: https://go-review.googlesource.com/38391
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The fmtmode and fmtpkgpfx globals stand in the
way of making the compiler more concurrent (#15756).
This CL removes them.
The natural way to eliminate a global is to explicitly
thread it as a parameter through all function calls.
However, most of the functions in gc/fmt.go
get called indirectly, by way of fmt format strings,
so there's nowhere natural to add a parameter.
Since there are only a few fmtmode modes,
use named types to distinguish between modes.
For example, fmtNodeErr, fmtNodeDbg, and fmtNodeTypeId
are all gc.Node, but they print in different modes.
Varying the type allows us to thread mode through fmt.
Handle fmtpkgpfx by converting it to a printing mode,
FTypeIdName, and using the same type-based approach.
To avoid a loss of readability and danger of bugs
from introducing conversions at all call sites,
instead add a helper that systematically modifies the args.
The only remaining gc/fmt.go global is dumpdepth.
Since that is used for debugging only,
it that can be handled with a global mutex,
or some similarly basic, if inefficient, protection.
Passes toolstash -cmp. No compiler performance impact.
For future reference, other options for threading state
that were considered and rejected:
* Wrapping values in structs, such as:
type fmtNode struct {
n *Node
mode fmtMode
}
This reduces the proliferation of types, and supports
easily adding extra local parameters.
However, putting such a struct into an interface{} allocates.
This is unacceptable in this particular area of code.
* Passing state via precision, such as:
fmt.Fprintf("%*v", mode, n)
where mode is the state encoded as an integer.
This avoids extra allocations, but it is out of keeping
with the intended semantics of precision, and is less readable.
* Modify the fmt package to support setting/getting context
via fmt.State. Unavailable due to Go 1 compatibility,
and probably the wrong solution anyway.
* Give up on package fmt. This would be a huge readability
regression and cause high code churn.
* Attempt a de-novo rewrite that circumvents these problems.
Too high a risk of bugs, with insufficient reward for the effort,
particularly since long term plans call for elimination
of gc.Node.
Change-Id: Iea2440d5a34a938e64273707de27e3a897cb41d1
Reviewed-on: https://go-review.googlesource.com/38147
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
There were a surprising number of places
in the tree that used yyerror for failed internal
consistency checks. Switch them to Fatalf.
Updates #15756
Updates #19250
Change-Id: Ie4278148185795a28ff3c27dacffc211cda5bbdd
Reviewed-on: https://go-review.googlesource.com/38153
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
After benchmarking with a compiler modified to have better
spill location, it became clear that this method of checking
was actually faster on (at least) two different architectures
(ppc64 and amd64) and it also provides more timely interruption
of loops.
This change adds a modified FOR loop node "FORUNTIL" that
checks after executing the loop body instead of before (i.e.,
always at least once). This ensures that a pointer past the
end of a slice or array is not made visible to the garbage
collector.
Without the rescheduling checks inserted, the restructured
loop from this change apparently provides a 1% geomean
improvement on PPC64 running the go1 benchmarks; the
improvement on AMD64 is only 0.12%.
Inserting the rescheduling check exposed some peculiar bug
with the ssa test code for s390x; this was updated based on
initial code actually generated for GOARCH=s390x to use
appropriate OpArg, OpAddr, and OpVarDef.
NaCl is disabled in testing.
Change-Id: Ieafaa9a61d2a583ad00968110ef3e7a441abca50
Reviewed-on: https://go-review.googlesource.com/36206
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Since switching to SSA, the only remaining use for the Ullman field
was in tracking whether or not an expression contained a function
call. Give it a new name and encode it in our fancy new bitset field.
Passes toolstash-check.
Change-Id: I95b7f9cb053856320c0d66efe14996667e6011c2
Reviewed-on: https://go-review.googlesource.com/37721
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
New special case for booleans and byte-sized integer types
converted to interfaces needs to ensure that the operand is
not too complex, if it were to appear in a parameter list
for example.
Added test, also increased the recursive node dump depth to
a level that was actually useful for an actual bug.
Fixes#19275.
Change-Id: If36ac3115edf439e886703f32d149ee0a46eb2a5
Reviewed-on: https://go-review.googlesource.com/37470
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
We can immediately emit static assignment data rather than queueing
them up to be processed during SSA building.
Passes toolstash -cmp.
Change-Id: I8bcea4b72eafb0cc0b849cd93e9cde9d84f30d5e
Reviewed-on: https://go-review.googlesource.com/37024
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Instead we can just call needwritebarrier when constructing the SSA
representation.
Change-Id: I6fefaad49daada9cdb3050f112889e49dca0047b
Reviewed-on: https://go-review.googlesource.com/34566
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Fixes#15055.
Updates exprfmt printing using fmt verb "%v" to check that n.Left
is non-nil before attempting to print it, otherwise we'll print
the nodes in the list using verb "%.v".
Credit to @mdempsky for this approach and for finding
the root cause of the issue.
Change-Id: I20a6464e916dc70d5565e145164bb9553e5d3865
Reviewed-on: https://go-review.googlesource.com/25361
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Known issue: #18640 (requires a bit more work, I believe).
For #18130.
Change-Id: I53dc26012070e0c79f63b7c76266732190a83d47
Reviewed-on: https://go-review.googlesource.com/35129
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Various minor adjustments.
Change-Id: Iedfb97989f7bedaa3e9e8993b167e05f162434a7
Reviewed-on: https://go-review.googlesource.com/34136
Reviewed-by: David Lazar <lazard@golang.org>
Adjust cmd/compile accordingly.
This will make it easier to replace the underlying implementation.
Change-Id: I33645850bb18c839b24785b6222a9e028617addb
Reviewed-on: https://go-review.googlesource.com/34133
Reviewed-by: David Lazar <lazard@golang.org>
EscScope behaves like EscHeap in current code.
There are no need to handle it specially.
So remove it and use EscHeap instead.
Change-Id: I910106fd147f00e5f4fd52c7dde05128141a5160
Reviewed-on: https://go-review.googlesource.com/32130
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
