This reverts commit 7365fac2db.
Reason for revert: breaks the build on some architectures, reading unmapped pages?
Change-Id: I3a8c02dc0b649269faacea79ecd8213defa97c54
Reviewed-on: https://go-review.googlesource.com/97995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
LLVM tools, particularly lldb and dsymutil, don't support base address
selection entries in location lists. When targeting GOOS=darwin,
mode, have the linker translate location lists to CU-relative form
instead.
Technically, this isn't necessary when linking internally, as long as
nobody plans to use anything other than Delve to look at the DWARF. But
someone might want to use lldb, and it's really confusing when dwarfdump
shows gibberish for the location entries. The performance cost isn't
noticeable, so enable it even for internal linking.
Doing this in the linker is a little weird, but it was more expensive in
the compiler, probably because the compiler is much more stressful to
the GC. Also, if we decide to only do it for external linking, the
compiler can't see the link mode.
Benchmark before and after this commit on Mac with -dwarflocationlists=1:
name old time/op new time/op delta
StdCmd 21.3s ± 1% 21.3s ± 1% ~ (p=0.310 n=27+27)
Only StdCmd is relevant, because only StdCmd runs the linker. Whatever
the cost is here, it's not very large.
Change-Id: Ic8ef780d0e263230ce6aa3ca3a32fc9abd750b1e
Reviewed-on: https://go-review.googlesource.com/97956
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Some SSA values don't translate into any instructions. If a function
began with two of them, and both modified the storage of the same
variable, we'd end up with a location list entry that started and ended
at 0. That looks like an end-of-list entry, which would then confuse
downstream tools, particularly the fixup in the linker.
"Fix" this by changing the end of such entries to 1. Should be harmless,
since AFAIK we don't generate any 1-byte instructions. Later CLs will
reduce the frequency of these entries anyway.
Change-Id: I9b7e5e69f914244cc826fb9f4a6acfe2dc695f81
Reviewed-on: https://go-review.googlesource.com/97955
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
In RET instruction, the operand is the return jump's target,
which should be put in Prog.To.
Add an action "buildrundir" to the test driver, which builds
(compile+assemble+link) the code in a directory and runs the
resulting binary.
Fixes#23838.
Change-Id: I7ebe7eda49024b40a69a24857322c5ca9c67babb
Reviewed-on: https://go-review.googlesource.com/94175
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
bytes.IndexByte is heavily optimized.
Use it in findnull.
name old time/op new time/op delta
GoString-8 65.5ns ± 1% 40.2ns ± 1% -38.62% (p=0.000 n=19+19)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: I2e6cb279c7d8078f8844065de684cc3567fe89d7
Reviewed-on: https://go-review.googlesource.com/97523
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Replace BYTE.. encodings with asm. This is possible due to asm
implementing more instructions and removal of
MOV $0, reg -> XOR reg, reg transformation from asm.
Change-Id: I011749ab6b3f64403ab6e746f3760c5841548b57
Reviewed-on: https://go-review.googlesource.com/97936
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
useSSE41 was used inside asm implementation of floor to select between base and ss4 code path.
We intrinsified floor and left asm functions as a backup for non-sse4 systems.
This made variable unused, so remove it.
Change-Id: Ia2633de7c7cb1ef1d5b15a2366b523e481b722d9
Reviewed-on: https://go-review.googlesource.com/97935
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Change-Id: I030baaa0a0abf1e43449faaf676d389a28a868a3
Reviewed-on: https://go-review.googlesource.com/97857
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
This avoid simple bugs like "ADD" matching "FADD". Obviously
"ADD" will still match "ADDQ" so some care is still required
in this regard, but at least a first class of possible errors
is taken care of.
Change-Id: I7deb04c31de30bedac9c026d9889ace4a1d2adcb
Reviewed-on: https://go-review.googlesource.com/97817
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
asmcheck comments now support a compact form of specifying
multiple checks for each platform, using the following syntax:
amd64:"SHL\t[$]4","SHR\t[$]4"
Negative checks are also parsed using the following syntax:
amd64:-"ROR"
though they are still not working.
Moreover, out-of-line comments have been implemented. This
allows to specify asmchecks on comment-only lines, that will
be matched on the first subsequent non-comment non-empty line.
// amd64:"XOR"
// arm:"EOR"
x ^= 1
Change-Id: I110c7462fc6a5c70fd4af0d42f516016ae7f2760
Reviewed-on: https://go-review.googlesource.com/97816
Reviewed-by: Keith Randall <khr@golang.org>
When the slice/string length is very large,
probably artifically large as in CL 97523,
adding BX (length) to R11 (pointer) overflows.
As a result, checking DI < R11 yields the wrong result.
Since they will be equal when the loop is done,
just check DI != R11 instead.
Yes, the pointer itself could overflow, but if that happens,
something else has gone pretty wrong; not our concern here.
Fixes#24187
Change-Id: I2f60fc6ccae739345d01bc80528560726ad4f8c6
Reviewed-on: https://go-review.googlesource.com/97802
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Don’t panic if a subtest inadvertently calls FailNow
on a parent’s T. Instead, report the offending subtest
while still reporting the error with the ancestor test and
keep exiting goroutines.
Note that this implementation has a race if parallel
subtests are failing the parent concurrently.
This is fine:
Calling FailNow on a parent is considered an error
in principle, at the moment, and is reported if it is
detected. Having the race allows the race detector
to detect the error as well.
Fixes#22882
Change-Id: Ifa6d5e55bb88f6bcbb562fc8c99f1f77e320015a
Reviewed-on: https://go-review.googlesource.com/97635
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
Reviewed-by: Kunpei Sakai <namusyaka@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The top-level test harness is modified to support a new kind
of test: "asmcheck". This is meant to replace asm_test.go
as an easier and more readable way to test code generation.
I've added a couple of codegen tests to get initial feedback
on the syntax. I've created them under a common "codegen"
subdirectory, so that it's easier to run them all with
"go run run.go -v codegen".
The asmcheck syntax allows to insert line comments that
can specify a regular expression to match in the assembly code,
for multiple architectures (the testsuite will automatically
build each testfile multiple times, one per mentioned architecture).
Negative matches are unsupported for now, so this cannot fully
replace asm_test yet.
Change-Id: Ifdbba389f01d55e63e73c99e5f5449e642101d55
Reviewed-on: https://go-review.googlesource.com/97355
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
All functions defined in syscall2_solaris.go have the respective libc_*
var in syscall_solaris.go, except for libc_close. Move it from
os3_solaris.go
Remove unused libc_fstat.
Order go:cgo_import_dynamic and go:linkname lists in
syscall2_solaris.go alphabetically.
Change-Id: I9f12fa473cf1ae351448ac45597c82a67d799c31
Reviewed-on: https://go-review.googlesource.com/97736
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Consider the following:
type child struct{ Field string }
type parent struct{ child }
p := new(parent)
v := reflect.ValueOf(p).Elem().Field(0)
v.Field(0).SetString("hello") // v.Field = "hello"
v = v.Addr().Elem() // v = *(&v)
v.Field(0).SetString("goodbye") // v.Field = "goodbye"
It would appear that v.Addr().Elem() should have the same value, and
that it would be safe to set "goodbye".
However, after CL 66331, any interspersed calls between Field calls
causes the RO flag to be set.
Thus, setting to "goodbye" actually causes a panic.
That CL affects decodeState.indirect which assumes that back-to-back
Value.Addr().Elem() is side-effect free. We fix that logic to keep
track of the Addr() and Elem() calls and set v back to the original
after a full round-trip has occured.
Fixes#24152
Updates #24153
Change-Id: Ie50f8fe963f00cef8515d89d1d5cbc43b76d9f9c
Reviewed-on: https://go-review.googlesource.com/97796
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instructions LDARB, LDARH, LDAXPW, LDAXP, STLRB, STLRH, STLXP, STLXPW, STXP,
STXPW have been added before, but they are not enabled. This CL enabled them.
Change the form of LDXP and LDXPW to the form of LDP, and fix a bug of STLXP.
Change-Id: I5d2b51494b92451bf6b072c65cfdd8acf07e9b54
Reviewed-on: https://go-review.googlesource.com/96215
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
runtime/trace test already skips tests in case of the timestamp
error.
Moreover, relax TestAnalyzeAnnotationGC test condition to
deal with the inaccuracy caused from use of cputicks in tracing.
Fixes#24081
Updates #16755
Change-Id: I708ecc6da202eaec07e431085a75d3dbfbf4cc06
Reviewed-on: https://go-review.googlesource.com/97757
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
OCOMPLIT stores the pre-typechecked type in n.Right, and then moves it
to n.Type. However, it wasn't clearing n.Right, so n.Right continued
to point to the OTYPE node. (Exception: slice literals reused n.Right
to store the array length.)
When exporting inline function bodies, we don't expect to need to save
any type aliases. Doing so wouldn't be wrong per se, but it's
completely unnecessary and would just bloat the export data.
However, reexportdep (whose role is to identify types needed by inline
function bodies) uses a generic tree traversal mechanism, which visits
n.Right even for O{ARRAY,MAP,STRUCT}LIT nodes. This means it finds the
OTYPE node, and mistakenly interpreted that the type alias needs to be
exported.
The straight forward fix is to just clear n.Right when typechecking
composite literals.
Fixes#24173.
Change-Id: Ia2d556bfdd806c83695b08e18b6cd71eff0772fc
Reviewed-on: https://go-review.googlesource.com/97719
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Otherwise, the error can be confusing if one forgets or doesn't know
that the builtin is being shadowed, which is not common practice.
Fixes#22822.
Change-Id: I735393b5ce28cb83815a1c3f7cd2e7bb5080a32d
Reviewed-on: https://go-review.googlesource.com/97455
Reviewed-by: Robert Griesemer <gri@golang.org>
Go 1.10 requires that SANs in certificates are valid. However, a
non-trivial number of (generally non-WebPKI) certificates have invalid
strings in dnsName fields and some have even put those dnsName SANs in
CA certificates.
This change defers validity checking until name constraints are checked.
Fixes#23995, #23711.
Change-Id: I2e0ebb0898c047874a3547226b71e3029333b7f1
Reviewed-on: https://go-review.googlesource.com/96378
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The tests checking for empty interfaces so that they can be fast-
tracked in the code actually didn't test the right field and the
fast track code never executed. Doing it now.
Change-Id: I58b2951efb3fb40b3366874c79fd653591ae0e99
Reviewed-on: https://go-review.googlesource.com/97519
Reviewed-by: Alan Donovan <adonovan@google.com>
In Plan 9, each M is implemented as a separate OS process with
its own working directory. To keep the wd consistent across
goroutines (or rescheduling of the same goroutine), CL 6350
introduced a Fixwd procedure which checks using getwd and calls
chdir if necessary before any syscall operating on a pathname.
This wd checking will not be necessary if the pathname is absolute
(starts with '/' or '#'). Getwd is a fairly expensive operation
in Plan 9 (implemented by opening "." and calling Fd2path on the
file descriptor). Eliminating the redundant getwd calls can
significantly reduce overhead for common operations like
"dist test --list" which perform many syscalls on absolute pathnames.
Updates #9428.
Change-Id: I13fd9380779de27b0ac2f2b488229778d6839255
Reviewed-on: https://go-review.googlesource.com/97675
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: David du Colombier <0intro@gmail.com>
Plan 9 won't let brk shrink the data segment if it's shared with
other processes (which it is in the go runtime). So we keep track
of the notional end of the segment as it moves up and down, and
call brk only when it grows.
Corrects CL 94776.
Updates #23860.
Fixes#24013.
Change-Id: I754232decab81dfd71d690f77ee6097a17d9be11
Reviewed-on: https://go-review.googlesource.com/97595
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change enables printing of relative column information if a
prior line directive specified a valid column. If there was no
line directive, or the line directive didn't specify a column
(or the -C flag is specified), no column information is shown in
file positions.
Implementation: Column values (and line values, for that matter)
that are zero are interpreted as "unknown". A line directive that
doesn't specify a column records that as a zero column in the
respective PosBase data structure. When computing relative columns,
a relative value is zero of the base's column value is zero.
When formatting a position, a zero column value is not printed.
To make this work without special cases, the PosBase for a file
is given a concrete (non-0:0) position 1:1 with the PosBase's
line and column also being 1:1. In other words, at the position
1:1 of a file, it's relative positions are starting with 1:1 as
one would expect.
In the package syntax, this requires self-recursive PosBases for
file bases, matching what cmd/internal/src.PosBase was already
doing. In src.PosBase, file and inlining bases also need to be
based at 1:1 to indicate "known" positions.
This change completes the cmd/compiler part of the issue below.
Fixes#22662.
Change-Id: I6c3d2dee26709581fba0d0261b1d12e93f1cba1a
Reviewed-on: https://go-review.googlesource.com/97375
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The first word of an interface is a pointer, but for the purposes
of GC we don't need to treat it as such.
1. If it is a non-empty interface, the pointer points to an itab
which is always in persistentalloc space.
2. If it is an empty interface, the pointer points to a _type.
a. If it is a compile-time-allocated type, it points into
the read-only data section.
b. If it is a reflect-allocated type, it points into the Go heap.
Reflect is responsible for keeping a reference to
the underlying type so it won't be GCd.
If we ever have a moving GC, we need to change this for 2b (as
well as scan itabs to update their itab._type fields).
Write barriers on the first word of interfaces have already been removed.
Change-Id: I643e91d7ac4de980ac2717436eff94097c65d959
Reviewed-on: https://go-review.googlesource.com/97518
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Make the hasSideEffects func use type information to see if a CallExpr
is a type conversion or not. In case it is, there cannot be any side
effects.
Now that vet always has type information, we can afford to use it here.
Update the tests and remove the TODO there too.
Change-Id: I74fdacf830aedf2371e67ba833802c414178caf1
Reviewed-on: https://go-review.googlesource.com/79536
Reviewed-by: Robert Griesemer <gri@golang.org>
Explicitly whitelist args of OpSelect{1|2} that zero upper 32 bits.
Use better values in corresponding test.
This should have been a part of CL 96815, but it was submitted, before
relevant comments.
Change-Id: Ic85d90a4471a17f6d64f8f5c405f21378bf3a30d
Reviewed-on: https://go-review.googlesource.com/97295
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
On amd64 we optimize encoding/binary.BigEndian.PutUint{16,32,64}
into bswap + single store, but strangely enough not LittleEndian.PutUint{16,32}.
We have similar rules, but they use 64-bit shifts everywhere,
and fail for 16/32-bit case. Add rules that matchLittleEndian.PutUint,
and relevant tests. Performance results:
LittleEndianPutUint16-6 1.43ns ± 0% 1.07ns ± 0% -25.17% (p=0.000 n=9+9)
LittleEndianPutUint32-6 2.14ns ± 0% 0.94ns ± 0% -56.07% (p=0.019 n=6+8)
LittleEndianPutUint16-6 1.40GB/s ± 0% 1.87GB/s ± 0% +33.24% (p=0.000 n=9+9)
LittleEndianPutUint32-6 1.87GB/s ± 0% 4.26GB/s ± 0% +128.54% (p=0.000 n=8+8)
Discovered, while looking at ethereum_ethash from community benchmarks
Change-Id: Id86d5443687ecddd2803edf3203dbdd1246f61fe
Reviewed-on: https://go-review.googlesource.com/95475
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
We accidentally overlooked needing to still visit Ninit for OIF
statements with constant conditions in golang.org/cl/96778.
Fixes#24120.
Change-Id: I5b341913065ff90e1163fb872b9e8d47e2a789d2
Reviewed-on: https://go-review.googlesource.com/97475
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
