parsePrivateKey can't return useful error messages because it does trial
decoding of multiple formats. Try ParseCertificate first in case it
offers a useful error message.
Fixes#23591
Change-Id: I380490a5850bee593a7d2f584a27b2a14153d768
Reviewed-on: https://go-review.googlesource.com/90435
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
1. STPW stores the lower 32-bit words of a pair of registers to memory.
2. LDPW loads two 32-bit words from memory, zero extends them to 64-bit,
and then copies to a pair of registers.
3. LDPSW does the same as LDPW, except a sign extension.
This CL implements those 3 instructions and adds test cases.
Change-Id: Ied9834d8240240d23ce00e086b4ea456e1611f1a
Reviewed-on: https://go-review.googlesource.com/99956
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Spans are represented using Async Event types of chrome trace viewer.
According to the doc, the 'id' should be unique within category, scope.
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.jh64i9l3vwa1
Use the index in the task's span slice as the slice id, so it
can be unique within the task. The scope is the task id which
is unique.
This fixes a visualization bug that caused incorrect or missing
presentation of nested spans.
Change-Id: If1537ee00247f71fa967abfe45569a9e7dbcdce7
Reviewed-on: https://go-review.googlesource.com/102697
Reviewed-by: Heschi Kreinick <heschi@google.com>
No need to sign extend input to Neg8 and Neg16.
Change-Id: I7896c83c9cdf84a34098582351a4aabf61cd6fdd
Reviewed-on: https://go-review.googlesource.com/102675
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In expandmeth, we call expand1/expand0 to build a list of all
candidate methods to promote, and then we use dotpath to prune down
which names actually resolve to a promoted method and how.
However, previously we still computed "followsptr" based on the
expand1/expand0 traversal (which is depth-first), rather than
dotpath (which is breadth-first). The result is that we could
sometimes end up miscomputing whether a particular promoted method
involves a pointer traversal, which could result in bad code
generation for method trampolines.
Fixes#24547.
Change-Id: I57dc014466d81c165b05d78b98610dc3765b7a90
Reviewed-on: https://go-review.googlesource.com/102618
Reviewed-by: Robert Griesemer <gri@golang.org>
Prevent queries from starting a goroutine if the context is
not able to be canceled.
Fixes#23879
Change-Id: I392047bd53d7f796219dd12ee11b07303658fdaf
Reviewed-on: https://go-review.googlesource.com/102478
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Yasuhiro MATSUMOTO <mattn.jp@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Decode AT_PAGESZ to determine physPageSize on netbsd.
Also rename vdso_none.go to auxv_none.go which matches its purpose more
closely.
Akin to CL 99780 which did the same for freebsd.
Change-Id: Iea4322f861ff0f3515e9051585dbb442f024326b
Reviewed-on: https://go-review.googlesource.com/102677
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change prevents import errors from being printed multiple times.
Creating a bare-bones package 'p' with only one file importing itself
and running 'go build p', the current implementation gives this error
message:
can't load package: import cycle not allowed
package p
imports p
import cycle not allowed
package p
imports p
With this change we will show the message only once.
Updates #23295
Change-Id: I653b34c1c06c279f3df514f12ec0b89745a7e64a
Reviewed-on: https://go-review.googlesource.com/86535
Reviewed-by: Harald Nordgren <haraldnordgren@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use the __vdso_clock_gettime fast path via the vDSO on linux/arm64 to
speed up nanotime and walltime. This results in the following
performance improvement for time.Now on Cavium ThunderX:
name old time/op new time/op delta
TimeNow 442ns ± 0% 163ns ± 0% -63.16% (p=0.000 n=10+10)
And benchmarks on VDSO
BenchmarkClockVDSOAndFallbackPaths/vDSO 10000000 166 ns/op
BenchmarkClockVDSOAndFallbackPaths/Fallback 3000000 456 ns/op
Change-Id: I326118c6dff865eaa0569fc45d1fc1ff95cb74f6
Reviewed-on: https://go-review.googlesource.com/99855
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
No changes in the actual generated compiler code.
Change-Id: I206a7bf7b60f70a73640119fc92974f79ed95a6b
Reviewed-on: https://go-review.googlesource.com/102416
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
CL 102457 added TestDevNullFile. However, this
test is failing on Plan 9, because it checks
that /dev/null is a character device while there
are no special files on Plan 9.
We fix this issue by changing Stat to consider
all files served by the console device (#c)
as character devices.
Fixes#24534.
Change-Id: I1c60cdf25770358b908790b3fb71910fa914dec0
Reviewed-on: https://go-review.googlesource.com/102424
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#24546
Change-Id: I99ebd5bc18e5c5e42eee4689644a7a8b02405f31
Reviewed-on: https://go-review.googlesource.com/102616
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I grepped for "bytes.Buffer" and "buf.String" and mostly ignored test
files. I skipped a few on purpose and probably missed a few others,
but otherwise I think this should be most of them.
Updates #18990
Change-Id: I5a6ae4296b87b416d8da02d7bfaf981d8cc14774
Reviewed-on: https://go-review.googlesource.com/102479
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The documentation was unclear here and I misremembered the behaviour and
changed it in 1.10: it used to be that matching any EKU was enough but
1.10 requires that all EKUs match.
Restore 1.9 behaviour and clarify the documentation to make it official.
Fixes#24162.
Change-Id: Ic9466cd0799cb27ec3a3a7e6c96f10c2aacc7020
Reviewed-on: https://go-review.googlesource.com/97720
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The way Value.AuxInt represents unsigned numbers is currently
documented in genericOps.go, which is not the most obvious place for
it. Move that documentation to Value.AuxInt. Furthermore, to make it
harder to use incorrectly, introduce a Value.AuxUnsigned accessor that
returns the zero-extended value of Value.AuxInt.
Change-Id: I85030c3c68761404058a430e0b1c7464591b2f42
Reviewed-on: https://go-review.googlesource.com/102597
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Even though GOARCH=sparc64 is not supported by gc (yet), it is easy to
make cgo already support it.
This e.g. allows to generate Go type definitions for linux/sparc64 in
the golang.org/x/sys/unix package without using gccgo.
Change-Id: I8886c81e7c895a0d93e350d81ed653fb59d95dd8
Reviewed-on: https://go-review.googlesource.com/102555
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Gdb can be sensitive to contents of .gdbinit, and to run
this test properly needs to have runtime/runtime-gdb.py
on the auto load safe path. Therefore, turn off .gdbinit
loading and explicitly add $GOROOT/runtime to the safe
load path.
This should make ssa/debug_test.go run more consistently.
Updates #24464.
Change-Id: I63ed17c032cb3773048713ce51fca3a3f86e79b6
Reviewed-on: https://go-review.googlesource.com/102598
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We already replaced most loops with PutOpBytesLit where possible,
do this in a last few places.
Change-Id: I8c90de017810145a12394fa6b887755e9111b22a
Reviewed-on: https://go-review.googlesource.com/102276
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
A newish check for branch-likely on single-successor blocks
caught a case where the preemption-check inserter was
setting "likely" on an unconditional branch.
Fixed by checking for that case before setting likely.
Also removed an overconservative restriction on parallel
compilation for GOEXPERIMENT=preemptibleloops; it works
fine, it is just another control-flow transformation.
Change-Id: I8e786e6281e0631cac8d80cff67bfb6402b4d225
Reviewed-on: https://go-review.googlesource.com/102317
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
We use 32-bit operations for 8- and 16-bit arithmetic, so use them
for comparisons too. This won't change performance but it is more
consistent and makes testing 8- and 16-bit comparison codegen
slightly more straightforward (for follow up CL).
Also fix a typo and add some additional double sign and zero
extension rules to remove the operations inserted by the comparison
rules.
Change-Id: I89ec1b0e09cb8be8090cf007be283ad88bba75a4
Reviewed-on: https://go-review.googlesource.com/102556
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was a typo mistake according to if cond and runtime/mheap.go:323
Change-Id: Id046d4afbfe0ea43cb29e1a9f400e1f130de221d
Reviewed-on: https://go-review.googlesource.com/102575
Reviewed-by: Austin Clements <austin@google.com>
This change makes errors in the example code a bit better, as it's no use to show the root dir when an error occurs walking a subdirectory or file.
Change-Id: I546276e9b151fabba5357258f03bfbd47a508201
GitHub-Last-Rev: 398c1eeb61
GitHub-Pull-Request: golang/go#24536
Reviewed-on: https://go-review.googlesource.com/102535
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add a note that if an error is returned after having read
at least the minimum no. of bytes, the error is set to nil.
Fixes#20477
Change-Id: I75ba5ee967be3ff80249e40d459da4afeeb53463
Reviewed-on: https://go-review.googlesource.com/102459
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Move part of UserSpan event processing from cmd/trace.analyzeAnnotations
to internal/trace.GoroutineStats that returns analyzed per-goroutine
execution information. Now the execution information includes list of
spans and their execution information.
cmd/trace.analyzeAnnotations utilizes the span execution information
from internal/trace.GoroutineStats and connects them with task
information.
Change-Id: Ib7f79a3ba652a4ae55cd81ea17565bcc7e241c5c
Reviewed-on: https://go-review.googlesource.com/101917
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Peter Weinberger <pjw@google.com>
- Summary: also includes links to pprof data.
- Sortable table: sorting is done on server-side. The intention is
that later, I want to add pagination feature and limit the page
size the browser has to handle.
- Stacked horizontal bar graph to present total time breakdown.
- Human-friendly time representation.
- No dependency on external fancy javascript libraries to allow
it to function without an internet connection.
Change-Id: I91e5c26746e59ad0329dfb61e096e11f768c7b73
Reviewed-on: https://go-review.googlesource.com/102156
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
CL 102456 added Lstat check to TestDevNullFile.
But some systems have /dev/null as a symlink,
so Lstat test is wrong. Remove the test.
Fixes#24521
Change-Id: I149110b08dd05db6495ec4eccbcf943e444332f9
Reviewed-on: https://go-review.googlesource.com/102461
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
RFC 8081 declares a top level font media type for all types of fonts.
Updating the mime types in sniffer to reflect the new changes.
Fixes#24524
Change-Id: Iba6cef4c5974e9930e14705720d42550ee87ba56
Reviewed-on: https://go-review.googlesource.com/102458
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also add more tests to test both nul and NUL on windows.
Fixes#24482
Change-Id: I3dfe68ec8de7f90ca869c1096dde0054df3c5cf6
Reviewed-on: https://go-review.googlesource.com/102457
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The build dashboard is dotted with net test failures.
We cannot declare all builders to have flaky networks,
although all fundamentally do.
Instead, add a simple retry/backoff loop to the ones that
show up most commonly on the dashboard at this moment.
If this approach works well in practice, we can
incrementally apply it to other flaky net tests.
Change-Id: I69c1ca6ce5b347ad549c7eb18d0438373f6e2489
Reviewed-on: https://go-review.googlesource.com/102397
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This aims to expand the coverage of examples showing how the sql
package works, as well as to address a number of issues I've observed
while explaining how the database package works:
- The best way to issue UPDATE or INSERT queries, that don't need
to scan anything in return. (Previously, we had no examples for any
Execute statement).
- How to use prepared statements and transactions.
- How to aggregate arguments from a Query/QueryContext query into
a slice.
Furthermore just having examples in more places should help, as users
click on e.g. the "Rows" return parameter and are treated with the
lack of any example about how Rows is used.
Switch package examples to use QueryContext/QueryRowContext; I think
it is a good practice to prepare users to issue queries with a timeout
attached, even if they are not using it immediately.
Change-Id: I4e63af91c7e4fff88b25f820906104ecefde4cc3
Reviewed-on: https://go-review.googlesource.com/91015
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
DevNull is documented on darwin, dragonfly, freebsd, linux,
nacl, netbsd, openbsd, solaris and plan9, but not on windows.
Add missing documentation.
Change-Id: Icdbded0dd5e322ed4360cbce6bee4cdca5cfbe72
Reviewed-on: https://go-review.googlesource.com/102456
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
As found by unparam. Picked the low-hanging fruit, consisting only of
errors that were always nil and results that were never used. Left out
those that were useful for consistency with other func signatures.
Change-Id: I06b52bbd3541f8a5d66659c909bd93cb3e172018
Reviewed-on: https://go-review.googlesource.com/102418
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Remove a couple of unnecessary var declarations, an unused sort.Sort
type, and simplify a range by using the two-name variant.
Change-Id: Ia251f634db0bfbe8b1d553b8659272ddbd13b2c3
Reviewed-on: https://go-review.googlesource.com/102336
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
And delete them from asm_test.
Change-Id: I34fcf85ae8ce09cd146fe4ce6a0ae7616bd97e2d
Reviewed-on: https://go-review.googlesource.com/102296
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
As pointed out by Josh Bleecher Snyder in CL 99780.
The check is for GOARM > 6, so suggest to recompile with either GOARM=5
or GOARM=6.
Change-Id: I6a97e87bdc17aa3932f5c8cb598bba85c3cf4be9
Reviewed-on: https://go-review.googlesource.com/101936
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Sometimes, we can end up calling update with a self-relation
about a variable (x REL x). In this case, there is no need
to record anything: the relation is unsatisfiable if and only
if it doesn't contain eq.
This also helps avoiding infinite loop in next CL that will
introduce transitive closure of relations.
Passes toolstash -cmp.
Change-Id: Ic408452ec1c13653f22ada35466ec98bc14aaa8e
Reviewed-on: https://go-review.googlesource.com/100276
Reviewed-by: Austin Clements <austin@google.com>
When an unsatisfiable relation is recorded in the facts table,
there is no need to compute further relations or updates
additional data structures.
Since we're about to transitively propagate relations, make
sure to fail as fast as possible to avoid doing useless work
in dead branches.
Passes toolstash -cmp.
Change-Id: I23eed376d62776824c33088163c7ac9620abce85
Reviewed-on: https://go-review.googlesource.com/100275
Reviewed-by: Austin Clements <austin@google.com>
The ssa.Func has Type field that is described as
function signature type.
It never gets any value and remains nil.
This leads to "<T>" signature printed representation.
Given this function declaration:
func foo(x int, f func() string) (int, error)
GOSSAFUNC printed it as below:
compiling foo
foo <T>
After this change:
compiling foo
foo func(int, func() string) (int, error)
Change-Id: Iec5eec8aac5c76ff184659e30f41b2f5fe86d329
Reviewed-on: https://go-review.googlesource.com/102375
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
By always writing out pack files, the object file format can be
simplified somewhat. In particular, the export data format will no
longer require escaping, because the pack file provides appropriate
framing.
This CL does not affect build systems that use -pack, which includes
all major Go build systems (cmd/go, gb, bazel).
Also, existing package import logic already distinguishes pack/object
files based on file contents rather than file extension.
The only exception is cmd/pack, which specially handled object files
created by cmd/compile when used with the 'c' mode. This mode is
extended to now recognize the pack files produced by cmd/compile and
handle them as before.
Passes toolstash-check.
Updates #21705.
Updates #24512.
Change-Id: Idf131013bfebd73a5cde7e087eb19964503a9422
Reviewed-on: https://go-review.googlesource.com/102236
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The __.PKGDEF file is a compiler object file only intended for other
compilers. Also, for build systems that use -linkobj, all of the
information it contains is present within the linker object files
already, so look for it there instead.
This requires a little bit of code reorganization. Significantly,
previously when loading an archive file, the __.PKGDEF file was
authoritative on whether the package was "main" and/or "safe". Now
that we're using the Go object files instead, there's the issue that
there can be multiple Go object files in an archive (because when
using assembly, each assembly file becomes its own additional object
file).
The solution taken here is to check if any object file within the
package declares itself as "main" and/or "safe".
Updates #24512.
Change-Id: I70243a293bdf34b8555c0bf1833f8933b2809449
Reviewed-on: https://go-review.googlesource.com/102281
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is currently always the case because loadobjfile complains if
it's not, but that will be changed soon.
Updates #24512.
Change-Id: I262daca765932a0f4cea3fcc1cc80ca90de07a59
Reviewed-on: https://go-review.googlesource.com/102280
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
AT_HWCAP is not available on FreeBSD-11.1-RELEASE or earlier and the wrong const was used.
Use the correct value, and initialize hwcap with ^uint32(0) inorder not to fail the VFP tests.
Fixes#24507.
Change-Id: I5c3eed57bb53bf992b7de0eec88ea959806306b9
Reviewed-on: https://go-review.googlesource.com/102355
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The compiler can't currently figure out that it can eliminate both c.s
loads (using store to load forwarding) in the second line of the
following code:
...
c.s[i], c.s[j] = c.s[j], c.s[i]
x := c.s[j] + c.s[i]
...
The compiler eliminates the second load of c.s[j] (using the original
value of c.s[i]), however the load of c.s[i] remains because the compiler
doesn't know that c.s[i] and c.s[j] either overlap completely or not at
all.
Introducing temporaries to make this explicit improves the performance
of the generic code slightly, the goal being to remove the assembly in
this package in the future. This change also hoists a bounds check out
of the main loop which gives a slight performance boost and also makes
the behaviour identical to the assembly implementation when len(dst) <
len(src).
name old speed new speed delta
RC4_128-4 491MB/s ± 3% 596MB/s ± 5% +21.51% (p=0.000 n=9+9)
RC4_1K-4 504MB/s ± 2% 616MB/s ± 1% +22.33% (p=0.000 n=10+10)
RC4_8K-4 509MB/s ± 1% 630MB/s ± 2% +23.85% (p=0.000 n=8+9)
Change-Id: I27adc775713b2e74a1a94e0c1de0909fb4379463
Reviewed-on: https://go-review.googlesource.com/102335
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit c6e69ec7f9.
Reason for revert: Broke builders. #24508
Change-Id: I66abff0dd14ec6e1f8d8d982ccfb0438633b639d
Reviewed-on: https://go-review.googlesource.com/102316
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
I don't know if I got lost in the old PKCS documents, or whether this is
a case where reality diverges from the spec, but OpenSSL clearly stuffs
PKIX Extension objects in CSR attributues directly[1].
In either case, doing what OpenSSL does seems valid here and allows the
critical flag in extensions to be serialised.
Fixes#13739.
[1] e3713c365c/crypto/x509/x509_req.c (L173)
Change-Id: Ic1e73ba9bd383a357a2aa8fc4f6bd76811bbefcc
Reviewed-on: https://go-review.googlesource.com/70851
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This change implement keying material export as described in:
https://tools.ietf.org/html/rfc5705
I verified the implementation against openssl s_client and openssl
s_server.
Change-Id: I4dcdd2fb929c63ab4e92054616beab6dae7b1c55
Signed-off-by: Mike Danese <mikedanese@google.com>
Reviewed-on: https://go-review.googlesource.com/85115
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Slightly simplifies the code. Made sure to exclude the cases that would
change behavior, such as when the iterated value is a string, when the
index is modified within the body, or when the slice is modified.
Also checked that all the elements are of pointer type, to avoid the
corner case where non-pointer types could be copied by mistake.
Change-Id: Iea64feb2a9a6a4c94ada9ff3ace40ee173505849
Reviewed-on: https://go-review.googlesource.com/100557
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This experiment has gone stale. It causes a type-checking failure
because the condition of the OIF produced by range loop lowering has
type "untyped bool". Fix this by typechecking the whole OIF statement,
not just its condition.
This doesn't quite fix the whole experiment, but it gets further.
Something about preemption point insertion is causing failures like
"internal compiler error: likeliness prediction 1 for block b10 with 1
successors" in cmd/compile/internal/gc.
Change-Id: I7d80d618d7c91c338bf5f2a8dc174d582a479df3
Reviewed-on: https://go-review.googlesource.com/102157
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Move currently uses mov instructions directly up to 31 bytes and then
switches to duffcopy. Moving 31 bytes is 4 instructions corresponding to
two loads and two stores, (or 6 if !useSSE) depending on the usage,
duffcopy is five (one or two mov, two or three lea, one call).
This adds direct mov instructions for Move's of size 32, 48, and 64 with
sse and for only size 32 without.
With useSSE:
- 32 is 4 instructions (byte +/- comparison below)
- 33 thru 48 is 6
- 49 thru 64 is 8
Without:
- 32 is 8
Note that the only platform with useSSE set to false is plan 9. I have
built three projects based off tip and tip with this patch and the
project's byte size is equal to or less than they were prior.
The basis of this change is that copying data with instructions directly
is nearly free, whereas calling into duffcopy adds a bit of overhead.
This is most noticeable in range statements where elements are 32+
bytes. For code with the following pattern:
func Benchmark32Range(b *testing.B) {
var f s32
for _, count := range []int{10, 100, 1000, 10000} {
name := strconv.Itoa(count)
b.Run(name, func(b *testing.B) {
base := make([]s32, count)
for i := 0; i < b.N; i++ {
for _, v := range base {
f = v
}
}
})
}
_ = f
}
These are the resulting benchmarks:
Benchmark16Range/10-4 19.1 19.1 +0.00%
Benchmark16Range/100-4 169 170 +0.59%
Benchmark16Range/1000-4 1684 1691 +0.42%
Benchmark16Range/10000-4 18147 18124 -0.13%
Benchmark31Range/10-4 141 142 +0.71%
Benchmark31Range/100-4 1407 1410 +0.21%
Benchmark31Range/1000-4 14070 14074 +0.03%
Benchmark31Range/10000-4 141781 141759 -0.02%
Benchmark32Range/10-4 71.4 32.2 -54.90%
Benchmark32Range/100-4 695 326 -53.09%
Benchmark32Range/1000-4 7166 3313 -53.77%
Benchmark32Range/10000-4 72571 35425 -51.19%
Benchmark64Range/10-4 87.8 64.9 -26.08%
Benchmark64Range/100-4 868 629 -27.53%
Benchmark64Range/1000-4 9355 6907 -26.17%
Benchmark64Range/10000-4 94463 70385 -25.49%
Benchmark79Range/10-4 177 152 -14.12%
Benchmark79Range/100-4 1769 1531 -13.45%
Benchmark79Range/1000-4 17893 15532 -13.20%
Benchmark79Range/10000-4 178947 155551 -13.07%
Benchmark80Range/10-4 99.6 99.7 +0.10%
Benchmark80Range/100-4 987 985 -0.20%
Benchmark80Range/1000-4 10573 10560 -0.12%
Benchmark80Range/10000-4 106792 106639 -0.14%
For runtime's BenchCopyFat* benchmarks:
CopyFat8-4 0.40ns ± 0% 0.40ns ± 0% ~ (all equal)
CopyFat12-4 0.40ns ± 0% 0.80ns ± 0% +100.00% (p=0.000 n=9+9)
CopyFat16-4 0.40ns ± 0% 0.80ns ± 0% +100.00% (p=0.000 n=10+8)
CopyFat24-4 0.80ns ± 0% 0.40ns ± 0% -50.00% (p=0.001 n=8+9)
CopyFat32-4 2.01ns ± 0% 0.40ns ± 0% -80.10% (p=0.000 n=8+8)
CopyFat64-4 2.87ns ± 0% 0.40ns ± 0% -86.07% (p=0.000 n=8+10)
CopyFat128-4 4.82ns ± 0% 4.82ns ± 0% ~ (p=1.000 n=8+8)
CopyFat256-4 8.83ns ± 0% 8.83ns ± 0% ~ (p=1.000 n=8+8)
CopyFat512-4 16.9ns ± 0% 16.9ns ± 0% ~ (all equal)
CopyFat520-4 14.6ns ± 0% 14.6ns ± 1% ~ (p=0.529 n=8+9)
CopyFat1024-4 32.9ns ± 0% 33.0ns ± 0% +0.20% (p=0.041 n=8+9)
Function calls are not benefitted as much due how they are compiled, but
other benchmarks I ran show that calling function with 64 byte elements
is marginally improved.
The main downside with this change is that it may increase binary sizes
depending on the size of the copy, but this change also decreases
binaries for moves of 48 bytes or less.
For the following code:
package main
type size [32]byte
//go:noinline
func use(t size) {
}
//go:noinline
func get() size {
var z size
return z
}
func main() {
var a size
use(a)
}
Changing size around gives the following assembly leading up to the call
(the initialization and actual call are removed):
tip func call with 32B arg: 27B
48 89 e7 mov %rsp,%rdi
48 8d 74 24 20 lea 0x20(%rsp),%rsi
48 89 6c 24 f0 mov %rbp,-0x10(%rsp)
48 8d 6c 24 f0 lea -0x10(%rsp),%rbp
e8 53 ab ff ff callq 448964 <runtime.duffcopy+0x364>
48 8b 6d 00 mov 0x0(%rbp),%rbp
modified: 19B (-8B)
0f 10 44 24 20 movups 0x20(%rsp),%xmm0
0f 11 04 24 movups %xmm0,(%rsp)
0f 10 44 24 30 movups 0x30(%rsp),%xmm0
0f 11 44 24 10 movups %xmm0,0x10(%rsp)
-
tip with 47B arg: 29B
48 8d 7c 24 0f lea 0xf(%rsp),%rdi
48 8d 74 24 40 lea 0x40(%rsp),%rsi
48 89 6c 24 f0 mov %rbp,-0x10(%rsp)
48 8d 6c 24 f0 lea -0x10(%rsp),%rbp
e8 43 ab ff ff callq 448964 <runtime.duffcopy+0x364>
48 8b 6d 00 mov 0x0(%rbp),%rbp
modified: 20B (-9B)
0f 10 44 24 40 movups 0x40(%rsp),%xmm0
0f 11 44 24 0f movups %xmm0,0xf(%rsp)
0f 10 44 24 50 movups 0x50(%rsp),%xmm0
0f 11 44 24 1f movups %xmm0,0x1f(%rsp)
-
tip with 64B arg: 27B
48 89 e7 mov %rsp,%rdi
48 8d 74 24 40 lea 0x40(%rsp),%rsi
48 89 6c 24 f0 mov %rbp,-0x10(%rsp)
48 8d 6c 24 f0 lea -0x10(%rsp),%rbp
e8 1f ab ff ff callq 448948 <runtime.duffcopy+0x348>
48 8b 6d 00 mov 0x0(%rbp),%rbp
modified: 39B [+12B]
0f 10 44 24 40 movups 0x40(%rsp),%xmm0
0f 11 04 24 movups %xmm0,(%rsp)
0f 10 44 24 50 movups 0x50(%rsp),%xmm0
0f 11 44 24 10 movups %xmm0,0x10(%rsp)
0f 10 44 24 60 movups 0x60(%rsp),%xmm0
0f 11 44 24 20 movups %xmm0,0x20(%rsp)
0f 10 44 24 70 movups 0x70(%rsp),%xmm0
0f 11 44 24 30 movups %xmm0,0x30(%rsp)
-
tip with 79B arg: 29B
48 8d 7c 24 0f lea 0xf(%rsp),%rdi
48 8d 74 24 60 lea 0x60(%rsp),%rsi
48 89 6c 24 f0 mov %rbp,-0x10(%rsp)
48 8d 6c 24 f0 lea -0x10(%rsp),%rbp
e8 09 ab ff ff callq 448948 <runtime.duffcopy+0x348>
48 8b 6d 00 mov 0x0(%rbp),%rbp
modified: 46B [+17B]
0f 10 44 24 60 movups 0x60(%rsp),%xmm0
0f 11 44 24 0f movups %xmm0,0xf(%rsp)
0f 10 44 24 70 movups 0x70(%rsp),%xmm0
0f 11 44 24 1f movups %xmm0,0x1f(%rsp)
0f 10 84 24 80 00 00 movups 0x80(%rsp),%xmm0
00
0f 11 44 24 2f movups %xmm0,0x2f(%rsp)
0f 10 84 24 90 00 00 movups 0x90(%rsp),%xmm0
00
0f 11 44 24 3f movups %xmm0,0x3f(%rsp)
So, at best we save 9B, at worst we gain 17. I do not think that copying
around 65+B sized types is common enough to bloat program sizes. Using
bincmp on the go binary itself shows a zero byte difference; there are
gains and losses all over. One of the largest gains in binary size comes
from cmd/go/internal/cache.(*Cache).Get, which passes around a 64 byte
sized type -- this is one of the cases I would expect to be benefitted
by this change.
I think that this marginal improvement in struct copying for 64 byte
structs is worth it: most data structs / work items I use in my programs
are small, but few are smaller than 32 bytes: with one slice, the budget
is up. The 32 rule alone would allow another 16 bytes, the 48 and 64
rules allow another 32 and 48.
Change-Id: I19a8f9190d5d41825091f17f268f4763bfc12a62
Reviewed-on: https://go-review.googlesource.com/100718
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-by: Keith Randall <khr@golang.org>
And remove them from asm_test.
Change-Id: I1ca29b40546d6de06f20bfd550ed8ff87f495454
Reviewed-on: https://go-review.googlesource.com/102115
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#14327
Much of the code is based on the linux/amd64 code that implements these
build modes, and code is shared where possible.
Change-Id: Ia510f2023768c0edbc863aebc585929ec593b332
Reviewed-on: https://go-review.googlesource.com/93875
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For each replacement, test case is added to new 386enc.s file
with exception of EMMS, SYSENTER, MFENCE and LFENCE as they
are already covered in amd64enc.s (same on amd64 and 386).
The replacement became less obvious after go vet suggested changes
Before:
BYTE $0x0f; BYTE $0x7f; BYTE $0x44; BYTE $0x24; BYTE $0x08
Changed to MOVQ (this form is being tested):
MOVQ M0, 8(SP)
Refactored to FP-relative access (go vet advice):
MOVQ M0, val+4(FP)
Change-Id: I56b87cf3371b6ad81ad0cd9db2033aee407b5818
Reviewed-on: https://go-review.googlesource.com/101475
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Decode AT_PAGESZ to determine physPageSize on freebsd/{386,amd64,arm}
and AT_HWCAP for hwcap and hardDiv on freebsd/arm. Also use hwcap to
perform the FP checks in checkgoarm akin to the linux/arm
implementation.
Change-Id: I532810a1581efe66277e4305cb234acdc79ee91e
Reviewed-on: https://go-review.googlesource.com/99780
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Otherwise, a populated GOPATH might result in failures such as:
$ go test
[...] no buildable Go source files in [...]/gopherjs/compiler/natives/src/crypto/rand
exit status 1
Move the initialization of the dirs walker out of the init func, so that
we can control its behavior in the tests.
Updates #24464.
Change-Id: I4b26a7d3d6809bdd8e9b6b0556d566e7855f80fe
Reviewed-on: https://go-review.googlesource.com/101836
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Unused variables in closures are currently not diagnosed by the
compiler (this is Issue #3059), while go/types catches them.
One unused variable in the cmd/trace tests is causing the go/types
test that typechecks the whole standard library to fail:
FAIL: TestStdlib (8.05s)
stdlib_test.go:223: cmd/trace/annotations_test.go:241:6: gcTime
declared but not used
FAIL
Remove it.
Updates #24464
Change-Id: I0f1b9db6ae1f0130616ee649bdbfdc91e38d2184
Reviewed-on: https://go-review.googlesource.com/101815
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The old code was a blend of (copied) code that existed before go/build,
and incorrect adjustments made when go/build was introduced. This change
leaves package path determination entirely to go/build and in the process
fixes issues with relative import paths.
Fixes#23092Fixes#24392
Change-Id: I9e900538b365398751bace56964495c5440ac4ae
Reviewed-on: https://go-review.googlesource.com/83415
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
VerifyHostname is called by tls.Conn during Handshake and does not need to be called explicitly.
Change-Id: I22b7fa137e76bb4be3d0018813a571acfb882219
Reviewed-on: https://go-review.googlesource.com/98618
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
It serialises optional parameters as empty rather than NULL. It's
probably technically correct, although ASN.1 has a long history of doing
this different ways.
But OpenSSL is likely common enough that we want to support this
encoding.
Fixes#23847
Change-Id: I81c60f0996edfecf59467dfdf75b0cf8ba7b1efb
Reviewed-on: https://go-review.googlesource.com/96417
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently we don't lift spill out of loop if loop contains call.
However often we have code like this:
for .. {
if hard_case {
call()
}
// simple case, without call
}
So instead of checking for any call, check for unavoidable call.
For #22698 cases I see:
mime/quotedprintable/Writer-6 10.9µs ± 4% 9.2µs ± 3% -15.02% (p=0.000 n=8+8)
And:
compress/flate/Encode/Twain/Huffman/1e4-6 99.4µs ± 6% 90.9µs ± 0% -8.57% (p=0.000 n=8+8)
compress/flate/Encode/Twain/Huffman/1e5-6 760µs ± 1% 725µs ± 1% -4.56% (p=0.000 n=8+8)
compress/flate/Encode/Twain/Huffman/1e6-6 7.55ms ± 0% 7.24ms ± 0% -4.07% (p=0.000 n=8+7)
There are no significant changes on go1 benchmarks.
But for cases with runtime arch checks, where we call generic version on old hardware,
there are respectable performance gains:
math/RoundToEven-6 1.43ns ± 0% 1.25ns ± 0% -12.59% (p=0.001 n=7+7)
math/bits/OnesCount64-6 1.60ns ± 1% 1.42ns ± 1% -11.32% (p=0.000 n=8+8)
Also on some runtime benchmarks loops have less loads and higher performance:
runtime/RuneIterate/range1/ASCII-6 15.6ns ± 1% 13.9ns ± 1% -10.74% (p=0.000 n=7+8)
runtime/ArrayEqual-6 3.22ns ± 0% 2.86ns ± 2% -11.06% (p=0.000 n=7+8)
Fixes#22698
Updates #22234
Change-Id: I0ae2f19787d07a9026f064366dedbe601bf7257a
Reviewed-on: https://go-review.googlesource.com/84055
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
And delete them from asm_test.
Change-Id: I64c512bfef3b3da6db5c5d29277675dade28b8ab
Reviewed-on: https://go-review.googlesource.com/101595
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Fix a bug in the code that generates the pre-inlined variable
declaration table used as raw material for emitting DWARF inline
routine records. The fix for issue 23704 altered the recipe for
assigning file/line/col to variables in one part of the compiler, but
didn't update a similar recipe in the code for variable tracking.
Added a new test that should catch problems of a similar nature.
Fixes#24460.
Change-Id: I255c036637f4151aa579c0e21d123fd413724d61
Reviewed-on: https://go-review.googlesource.com/101676
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The atomic add instructions modify the condition code and so need to
be marked as clobbering flags.
Fixes#24449.
Change-Id: Ic69c8d775fbdbfb2a56c5e0cfca7a49c0d7f6897
Reviewed-on: https://go-review.googlesource.com/101455
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change fixes index error when encoding VMOV instruction which pattern
is vmov Vn.<T>[index], Vd.<T>[index]
Change-Id: I949166e6dfd63fb0a9365f183b6c50d452614f9d
Reviewed-on: https://go-review.googlesource.com/101335
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This change fixes index error when encoding VMOV instruction which pattern is
VMOV Rn, V.<T>[index]. For example VMOV R1, V1.S[1] is assembled as VMOV R1, V1.S[0]
Fixes#24400
Change-Id: I82b5edc8af4e06862bc4692b119697c6bb7dc3fb
Reviewed-on: https://go-review.googlesource.com/101297
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This reverts commit bfa8b6f8ff.
Reason for revert: This depends on another CL which is not yet submitted.
Change-Id: I50e7594f1473c911a2079fe910849a6694ac6c07
Reviewed-on: https://go-review.googlesource.com/101496
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Current ARM64 assembler has no check for the invalid value of both
shift amount and post-index immediate offset of LD1/ST1. This patch
adds the check.
This patch also fixes the printing error of register number equals
to 31, which should be printed as ZR instead of R31. Test cases
are also added.
Change-Id: I476235f3ab3a3fc91fe89c5a3149a4d4529c05c7
Reviewed-on: https://go-review.googlesource.com/100255
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
One could expect calls like
z.mant.shl(z.mant, shiftAmount)
(or higher-level-functions calls that use lhs/rhs) to be almost free
when shiftAmount = 0; and expect calls like
z.mant.shl(x.mant, 0)
to have the same cost of a x.mant -> z.mant copy. Neither of this
things are currently true.
For an 800 words nat, the first kind of calls cost ~800ns for rigth
shifts and ~3.5µs for left shift; while the second kind of calls are
doing more work than necessary by calling shlVU/shrVU.
This change makes the first kind of calls ({Shl,Shr}Same) almost free,
and the second kind of calls ({Shl,Shr}) about 30% faster.
name old time/op new time/op delta
ZeroShifts/Shl-4 3.64µs ± 3% 2.49µs ± 1% -31.55% (p=0.000 n=10+10)
ZeroShifts/ShlSame-4 3.65µs ± 1% 0.01µs ± 1% -99.85% (p=0.000 n=9+9)
ZeroShifts/Shr-4 3.65µs ± 1% 2.49µs ± 1% -31.91% (p=0.000 n=10+10)
ZeroShifts/ShrSame-4 825ns ± 0% 6ns ± 1% -99.33% (p=0.000 n=9+10)
During go test math/big, the shl zeroshift fastpath is triggered 1380
times; while the shr fastpath is triggered 153334 times(!).
Change-Id: I5f92b304a40638bd8453a86c87c58e54b337bcdf
Reviewed-on: https://go-review.googlesource.com/87660
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Per #11257 all examples should be in external test files.
Additionally, doing so makes this example playable.
Updates #24352. (Albeit tangentially).
Change-Id: I77ab4655107f61db2e9d21a608b73ace3a230fb2
Reviewed-on: https://go-review.googlesource.com/101285
Reviewed-by: Robert Griesemer <gri@golang.org>
And delete them from asm_test.
Change-Id: I3cf0934706a640136cb0f646509174f8c1bf3363
Reviewed-on: https://go-review.googlesource.com/101395
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Replaces " \t" code indentation with "\t".
Issues like this are easy to spot with editor that prints
whitespace charecters.
Change-Id: Ia82877e7c99121bf369fa76e46ba52dff84f36bf
Reviewed-on: https://go-review.googlesource.com/101355
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
By moving exported methods to the front of method lists, filtering
down to only the exported methods just needs a count of how many
exported methods exist, which the compiler can statically
provide. This allows getting rid of the exported method cache.
For #22075.
Change-Id: I8eeb274563a2940e1347c34d673f843ae2569064
Reviewed-on: https://go-review.googlesource.com/100846
Reviewed-by: Ian Lance Taylor <iant@golang.org>
By sorting method sets earlier, we can change the interface
satisfaction problem from taking O(NM) time to O(N+M). This is the
same algorithm already used by runtime and reflect for dynamic
interface satisfaction testing.
For #22075.
Change-Id: I3d889f0227f37704535739bbde11f5107b4eea17
Reviewed-on: https://go-review.googlesource.com/100845
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Add a search box to the top of the user task views that only displays
tasks containing a particular log message.
Change-Id: I92f4aa113f930954e8811416901e37824f0eb884
Reviewed-on: https://go-review.googlesource.com/100843
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Goroutine analysis reports the sum of all overlapping GC intervals as
the GCTime of a goroutine. The computation is done by adding the length
of a completed GC interval to 'active' goroutines when processing the
corresponding EvGCDone event. This change fixes the two corner cases
the current implementation ignores:
1) Goroutine that ends during GC. Previously, this goroutine was ignored
and GC time was undercounted. We handle this case by setting the
gcStartTime only when GC is active and handling non-zero gcStartTime
when processing EvGoStop and EvGoStart.
2) Goroutine that starts during GC. Previously, the entire GC interval
length was added to the Goroutine's GCTime which resulted in overcount
of GC time. We handle this case by computing the length of overlapped
period precisely.
Change-Id: Ifa8e82672ec341b5ff87837209f4311fa7262b7f
Reviewed-on: https://go-review.googlesource.com/100842
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
And delete them from asm_test.
Change-Id: Ibdaca3496eefc73c731b511ddb9636a1f3dff68c
Reviewed-on: https://go-review.googlesource.com/100915
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Based on decision for #24183. This makes the go/scanner behavior
match cmd/compile behavior. Adjusted a go/printer test that assumed
silent behavior for invalid line directive, and added more scanner
tests verifying the correct error position and message for invalid
line directives.
The filenames in line directives now remain untouched by the scanner;
there is no cleanup or conversion of relative into absolute paths
anymore, in sync with what the compiler's scanner/parser are doing.
Any kind of filename transformation has to be done by a client. This
makes the scanner code simpler and also more predictable.
For #24183.
Change-Id: Ia091548e1d3d89dfdf6e7d82dab50bea05742ce3
Reviewed-on: https://go-review.googlesource.com/100235
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When there are plugins, there may not be a unique copy of runtime
functions like goexit, mcall, etc. So identifying them by entry
address is problematic. Instead, keep track of each special function
using a field in the symbol table. That way, multiple copies of
the same runtime function will be treated identically.
Fixes#24351Fixes#23133
Change-Id: Iea3232df8a6af68509769d9ca618f530cc0f84fd
Reviewed-on: https://go-review.googlesource.com/100739
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There is a bug in Octeon III processors where storing an odd floating
point register after it has recently been written to by a double
floating point operation will store the old value from before the double
operation (there are some extra details - the operation and store
must be a certain number of cycles apart). However, this bug does not
occur if the even register is stored first. Currently the bug only
happens on big endian because go always loads the even register first on
little endian.
Workaround the bug by always loading / storing the even floating point
register first. Since this is just an instruction reordering, it should
have no performance penalty. This follows other compilers like GCC which
will always store the even register first (although you do have to set
the ISA level to MIPS I to prevent it from using SDC1).
Change-Id: I5e73daa4d724ca1df7bf5228aab19f53f26a4976
Reviewed-on: https://go-review.googlesource.com/97735
Reviewed-by: Keith Randall <khr@golang.org>
And delete them from asm_test.
Change-Id: I29c8d098a8893e6b669b6272a2f508985ac9d618
Reviewed-on: https://go-review.googlesource.com/100876
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The Android O seccomp policy disallows the stat syscall on amd64, see
https://android.googlesource.com/platform/bionic/+/android-4.2.2_r1.2/libc/SYSCALLS.TXT
Use the fstatat syscall with AT_FDCWD and zero flags instead to achieve
the same behavior.
Fixes#24403
Change-Id: I36fc9ec9bc938cd8e9de30f66c0eb9d2e24debf6
Reviewed-on: https://go-review.googlesource.com/100878
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The struct stores its 64-bit state field in a 12-byte array to
ensure that it can be 64-bit-aligned. This leaves 4 spare bytes,
which we can reuse to store the sema field.
(32-bit alignment is still guaranteed because the array type was
changed to [3]uint32.)
Fixes#19149.
Change-Id: I9bc20e69e45e0e07fbf496080f3650e8be0d6e8d
Reviewed-on: https://go-review.googlesource.com/100515
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When combining adjacent type switch cases with the same type hash, we
failed to actually remove the combined cases, so we would generate
code for them twice.
We use MD5 for type hashes, so collisions are rare, but they do
currently appear in test/fixedbugs/bug248.dir/bug2.go, which is how I
noticed this failure.
Passes toolstash-check.
Change-Id: I66729b3366b96cb8ddc8fa6f3ebea11ef6d74012
Reviewed-on: https://go-review.googlesource.com/100461
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The main thing is we now eagerly create the ODCLFUNC node for
closures, immediately cross-link them, and assign fields (e.g., Nbody,
Dcl, Parents, Marks) directly on the ODCLFUNC (previously they were
assigned on the OCLOSURE and later moved to the ODCLFUNC).
This allows us to set Curfn to the ODCLFUNC instead of the OCLOSURE,
which makes things more consistent with normal function declarations.
(Notably, this means Cvars now hang off the ODCLFUNC instead of the
OCLOSURE.)
Assignment of xfunc symbol names also now happens before typechecking
their body, which means debugging output now provides a more helpful
name than "<S>".
In golang.org/cl/66810, we changed "x := y" statements to avoid
creating false closure variables for x, but we still create them for
struct literals like "s{f: x}". Update comment in capturevars
accordingly.
More opportunity for cleanups still, but this makes some substantial
progress, IMO.
Passes toolstash-check.
Change-Id: I65a4efc91886e3dcd1000561348af88297775cd7
Reviewed-on: https://go-review.googlesource.com/100197
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This reverts commit 080187f4f7.
It broke build of golang.org/x/exp/shiny/iconvg
See issue 24395 for details
Change-Id: Ifd6134f6214e6cee40bd3c63c32941d5fc96ae8b
Reviewed-on: https://go-review.googlesource.com/100755
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
User variables that cannot be SSA'd, either because their addresses are
taken or because they are too large for the decomposition heuristic, do
not explicitly appear as operands of SSA values. Instead they are written
to directly via the stack pointer.
This hid them from the location list generation, which is only
interested in the named value table. Fortunately, the lifetime of
stack-only variables is delineated by VarDef/VarKill ops, and it's easy
enough to turn those into location list bounds.
One wrinkle: stack frame information is not explicitly available in the
SSA phases, because it's owned by the frontend in AllocFrame. It would
be easier if the set of live LocalSlots were returned by that, but this
is the minimal change to fix missing variables. Or VarDef/VarKills
could appear in NamedValues, which would make this change even easier.
Change-Id: Ice6654dad6f9babb0286e95c7ec28594561dc91f
Reviewed-on: https://go-review.googlesource.com/100458
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some rules in PPC64.rules cause an extremely large rewritePPC64.go
file to be generated, due to rules with commutative operations and
many operands. This happens with the existing
rules for combining byte loads in little endian order, and
also happens with the pending change to do the same for bytes
in big endian order.
The change improves the existing rules and reduces the size of
the rewrite file by more than 60%. Once this change is merged,
then the pending change for big endian ordered rules will be
updated to use rules that avoid generating an excessively large
rewrite file.
This also includes a fix to a performance regression for
littleEndian.PutUint16 on ppc64le.
Change-Id: I8d2ea42885fa2b84b30c63aa124b0a9b130564ff
Reviewed-on: https://go-review.googlesource.com/100675
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The syscall package currently declares RawSyscall6 for every GOOS, but
does not define it on Solaris. This leads to code using said function
to compile but it will not link. Fix it by adding RawSyscall6 and make
it panic.
Also remove the obsolete comment above runtime.syscall_syscall as
pointed out by Aram.
Updates #24357
Change-Id: I1b1423121d1c99de2ecc61cd9a935dba9b39e3a4
Reviewed-on: https://go-review.googlesource.com/100655
Reviewed-by: Aram Hăvărneanu <aram@mgk.ro>
This change ports all the remaining tests checking that small memmoves
are replaced with MOVs to the new codegen test harness, and deletes
them from the asm_test file.
Change-Id: I01c94b441e27a5d61518035af62d62779dafeb56
Reviewed-on: https://go-review.googlesource.com/100476
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Thanks to Iskander Sharipov for spotting this in an earlier CL of mine.
Change-Id: Idf45ad266205ff83985367cb38f585badfbed151
Reviewed-on: https://go-review.googlesource.com/100535
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Iskander Sharipov <iskander.sharipov@intel.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In CL 98015, findnull was rewritten so it uses bytes.IndexByte.
This broke the build on plan9/amd64 because the implementation
of bytes.IndexByte on AMD64 relies on SSE instructions while
floating point instructions are not allowed in the note handler.
This change fixes findnull by using the former implementation
on Plan 9, so it doesn't use bytes.IndexByte.
Fixes#24387.
Change-Id: I084d1a44d38d9f77a6c1ad492773f0a98226be16
Reviewed-on: https://go-review.googlesource.com/100577
Run-TryBot: David du Colombier <0intro@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This rule is meant for code optimization, but it makes other rules
potentially more complex, as they need to cope with the fact that
a 32-bit op (BTLconst) can appear everywhere a 64-bit rule maches.
Move the optimization to opcode expansion instead. Tests will be
added in following CL.
Change-Id: Ica5ef291e7963c4af17c124d4a2869e6c8f7b0c7
Reviewed-on: https://go-review.googlesource.com/99995
Reviewed-by: Keith Randall <khr@golang.org>
Only the contiguous ones, to keep the patch simple. Remove some
unnecessary newlines, while at it.
Change-Id: Ia588f80538b49a169fbf49835979ebff5a0a7b6d
Reviewed-on: https://go-review.googlesource.com/94756
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The imm8 argument consists of 4 2-bit indices, so it can take values up
to $255. However, the assembler was treating it as Yi8, which reads
"fits in int8". Add a Yu8 variant, to also keep backwards compatibility
with negative values possible with Yi8.
Fixes#24378.
Change-Id: I24ddb19c219b54d039a6c1bcdb903717d1c7c3b8
Reviewed-on: https://go-review.googlesource.com/100475
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
As a side effect of working on mid-stack inlining, we've fixed support
for inlining variadic functions. Might as well enable it.
Change-Id: I7f555f8b941969791db7eb598c0b49f6dc0820aa
Reviewed-on: https://go-review.googlesource.com/100456
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
These are always set to n.Isddd(), which is readily available within
mkinlcall.
Change-Id: I3d7fbc9dc19a40d6b905691c666eee9bcd031a00
Reviewed-on: https://go-review.googlesource.com/100455
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The current implementation of l*arx instructions does not accept non-zero
offsets in RA nor the EH field. This change adds full functionality to those
instructions.
Updates #23845
Change-Id: If113f70d11de5f35f8389520b049390dbc40e863
Reviewed-on: https://go-review.googlesource.com/99635
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
As reported by unparam.
Passes toolstash -cmp on std cmd.
Change-Id: I55473e1eed096ed1c3e431aed2cbf0b6b5444b91
Reviewed-on: https://go-review.googlesource.com/97895
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Logical instructions can have RSP as its destination. Support it.
Note that the two-operand form, like "AND $1, RSP", which is
equivalent to the three-operand form "AND $1, RSP, RSP", is
invalid, because the source register is not allowed to be RSP.
Also note that instructions that set the conditional flags, like
ANDS, cannot target RSP. Because of this, we split out the optab
entries of AND et al. and ANDS et al.
Merge the optab entries of BIC et al. to AND et al., because they
are same.
Fixes#24332.
Change-Id: I3584d6f2e7cea98a659a1ed9fdf67c353e090637
Reviewed-on: https://go-review.googlesource.com/100217
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The compiler now emits TBZ like instructions, but the assembler's
too-far-branch patch code didn't include that case. Add it.
Fixes#23889.
Change-Id: Ib75f9250c660b9fb652835fbc83263a5d5073dc5
Reviewed-on: https://go-review.googlesource.com/94902
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is the "3rd bug" that caused compilations to sometimes
produce different results when dwarf location lists were
enabled.
A loop had not been properly rewritten in an earlier
optimization CL, and it accessed uninitialized data,
which was deterministically perhaps wrong when single
threaded, but variably wrong when multithreaded.
Change-Id: Ib3da538762fdf7d5e4407106f2434f3b14a1d7ea
Reviewed-on: https://go-review.googlesource.com/99935
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
There are few uses for the majority of the API in go/token for the
average user. The exception to this is getting the filename, line, and
column information from a token.Pos (reported and absolute. This is
straightforward but figuring out how to do it requires combing through
a lot of documentation. This example makes it more easily discoverable.
Updates #24352.
Change-Id: I0a45da6173b3dabebf42484bbbed30d9e5e20e01
Reviewed-on: https://go-review.googlesource.com/100058
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
Change-Id: I7ffbd52fbc2a286931a2218038fda52ed6473cc9
Reviewed-on: https://go-review.googlesource.com/99078
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Generated by running gofmt -s on the files in question.
Change-Id: If6578b150e1bfced8657196d2af01f5d36879f93
Reviewed-on: https://go-review.googlesource.com/100135
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, order desugars map assignment operations like
m[k] op= r
into
m[k] = m[k] op r
which in turn is transformed during walk into:
tmp := *mapaccess(m, k)
tmp = tmp op r
*mapassign(m, k) = tmp
However, this is suboptimal, as we could instead produce just:
*mapassign(m, k) op= r
One complication though is if "r == 0", then "m[k] /= r" and "m[k] %=
r" will panic, and they need to do so *before* calling mapassign,
otherwise we may insert a new zero-value element into the map.
It would be spec compliant to just emit the "r != 0" check before
calling mapassign (see #23735), but currently these checks aren't
generated until SSA construction. For now, it's simpler to continue
desugaring /= and %= into two map indexing operations.
Fixes#23661.
Change-Id: I46e3739d9adef10e92b46fdd78b88d5aabe68952
Reviewed-on: https://go-review.googlesource.com/91557
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
cmd/asm now supports three-operand form of IMUL,
so instead of using IMUL with resultInArg0, emit IMUL3 instruction.
This results in less redundant MOVs where SSA assigns
different registers to input[0] and dst arguments.
Note: these have exactly the same encoding when reg0=reg1:
IMUL3x $const, reg0, reg1
IMULx $const, reg
Two-operand IMULx is like a crippled IMUL3x, with dst fixed to input[0].
This is why we don't bother to generate IMULx for the case where
dst is the same as input[0].
Change-Id: I4becda475b3dffdd07b6fdf1c75bacc82af654e4
Reviewed-on: https://go-review.googlesource.com/99656
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Keith Randall <khr@golang.org>
The current code assigns vector register arrangement a wrong value
when the arrangement specifier is S2, which causes the incorrect
assembly.
The patch fixes the issue and adds the test cases.
Fixes#24249
Change-Id: I9736df1279494003d0b178da1af9cee9cd85ce21
Reviewed-on: https://go-review.googlesource.com/98555
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Remove old tests from asm_test.
Change-Id: Ib408ec7faa60068bddecf709b93ce308e0ef665a
Reviewed-on: https://go-review.googlesource.com/100075
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
The tool has gotten better over time, so re-generating the files brings
some advantages like fewer objects, dropping the use of fmt, and
dropping unnecessary bounds checks.
While at it, add the missing go:generate line for obj.AddrType.
Change-Id: I120c9795ee8faddf5961ff0384b9dcaf58d831ff
Reviewed-on: https://go-review.googlesource.com/100015
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I found files to change with this command:
git grep 'DO NOT EDIT' | grep -v 'Code generated .* DO NOT'
There are more files that match that grep, but I do not intend on fixing
them.
Change-Id: I4b474f1c29ca3135560d414785b0dbe0d1a4e52c
GitHub-Last-Rev: 65804b0263
GitHub-Pull-Request: golang/go#24334
Reviewed-on: https://go-review.googlesource.com/99955
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The "updates" lines, such as RUN, do not contain a colon. However,
test2json looked for one anyway, meaning that it would be thrown off if
it encountered a line like:
=== RUN TestWithColons/[::1]
In that case, it must not use the first colon it encounters to separate
the action from the test name.
Fixes#23920.
Change-Id: I82eff23e24b83dae183c0cf9f85fc5f409f51c25
Reviewed-on: https://go-review.googlesource.com/98445
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Added examples for use of Mode, Whitespace, and IsIdentRune properties.
Fixes#23768
Change-Id: I2528e14fde63a4476f3c25510bf0c5b73f38ba5d
Reviewed-on: https://go-review.googlesource.com/93199
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Before DWARF location lists can be turned on, 3 bugs need
fixing.
This CL addresses two -- lack of register definitions for
various architectures, and bugs on 32-bit platforms.
The third bug comes later.
Passes
GO_GCFLAGS=-dwarflocationlists ./run.bash -no-rebuild
(-no-rebuild because the map dependence causes trouble)
Change-Id: I4223b48ade84763e4b048e4aeb81149f082c7bc7
Reviewed-on: https://go-review.googlesource.com/99255
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This change updates go/scanner to recognize the extended line
directives that are now also handled by cmd/compile:
//line filename:line
//line filename:line:column
/*line filename:line*/
/*line filename:line:column*/
As before, //-style line directives must start in column 1.
/*-style line directives may be placed anywhere in the code.
In both cases, the specified position applies to the character
immediately following the comment; for line comments that is
the first character on the next line (after the newline of the
comment).
The go/token API is extended by a new method
File.AddLineColumnInfo(offset int, filename string, line, column int)
which extends the existing
File.AddLineInfo(offset int, filename string, line int)
by adding a column parameter.
Adjusted token.Position computation is changed to take into account
column information if provided via a line directive: A (line-directive)
relative position will have a non-zero column iff the line directive
specified a column; if the position is on the same line as the line
directive, the column is relative to the specified column (otherwise
it is relative to the line beginning). See also #24183.
Finally, Position.String() has been adjusted to not print a column
value if the column is unknown (== 0).
Fixes#24143.
Change-Id: I5518c825ad94443365c049a95677407b46ba55a1
Reviewed-on: https://go-review.googlesource.com/97795
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are two less optimized SSA rules in my previous CL
https://go-review.googlesource.com/c/go/+/95075 .
This CL fixes that issue and a test case gets about 10%
performance improvement.
name old time/op new time/op delta
MNEG-4 263µs ± 3% 235µs ± 3% -10.53% (p=0.000 n=20+20)
(https://github.com/benshi001/ugo1/blob/master/mneg_7_test.go)
Change-Id: I30087097e281dd9d9d1c870d32e13b4ef4a96ad3
Reviewed-on: https://go-review.googlesource.com/99495
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The implementation of runtime.abort on arm64 currently branches to
address 0, which results in a signal from PC 0, rather than from
runtime.abort, so the runtime fails to recognize it as an abort.
Fix runtime.abort on arm64 to read from address 0 like what other
architectures do and recognize this in the signal handler.
Should fix the linux/arm64 build.
Change-Id: I960ab630daaeadc9190287604d4d8337b1ea3853
Reviewed-on: https://go-review.googlesource.com/99895
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
bytes.IndexByte is heavily optimized. Use it in findnull.
This is second attempt, similar to CL97523.
In this version we never call IndexByte on region of memory,
that crosses page boundary. A bit slower than CL97523,
but still fast:
name old time/op new time/op delta
GoString-6 164ns ± 2% 118ns ± 0% -28.00% (p=0.000 n=10+6)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: Id843dd4f65a34309d92bdd8df229e484d26b0cb2
Reviewed-on: https://go-review.googlesource.com/98015
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This lets SIGPROF signals get a useful traceback.
Without it we just see sysvicallN calling asmcgocall.
Updates #24142
Change-Id: I5dfe3add51f0c3a4cb1c98acb7738be6396214bc
Reviewed-on: https://go-review.googlesource.com/99617
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The index 248 results in the decoder calling reflect.MakeMapWithSize
with a size of 14754407682 - just under 15GB - which ends up in a
runtime out of memory panic after some recent runtime changes on
machines with 8GB of memory.
Until that is fixed in either runtime or gob, skip the troublesome
index.
Updates #24308.
Change-Id: Ia450217271c983e7386ba2f3f88c9ba50aa346f4
Reviewed-on: https://go-review.googlesource.com/99655
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Borrowed from cmd/compile, TestSizeof ensures
that the size of important types doesn't change unexpectedly.
It also helps reviewers see the impact of intended changes.
Change-Id: If57955f0c3e66054de3f40c6bba585b88694c7be
Reviewed-on: https://go-review.googlesource.com/99837
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is optimization is only for IPv4. It allocates a result buffer and
writes the IPv4 octets as dotted decimal into it before converting
it to a string just once, reducing allocations.
Benchmark shows performance improvement:
name old time/op new time/op delta
IPString/IPv4-8 284ns ± 4% 144ns ± 6% -49.35% (p=0.000 n=19+17)
IPString/IPv6-8 1.34µs ± 5% 1.14µs ± 5% -14.37% (p=0.000 n=19+20)
name old alloc/op new alloc/op delta
IPString/IPv4-8 24.0B ± 0% 16.0B ± 0% -33.33% (p=0.000 n=20+20)
IPString/IPv6-8 232B ± 0% 224B ± 0% -3.45% (p=0.000 n=20+20)
name old allocs/op new allocs/op delta
IPString/IPv4-8 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=20+20)
IPString/IPv6-8 12.0 ± 0% 11.0 ± 0% -8.33% (p=0.000 n=20+20)
Fixes#24306
Change-Id: I4e2d30d364e78183d55a42907d277744494b6df3
Reviewed-on: https://go-review.googlesource.com/99395
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
And delete them from asm_go.
Change-Id: I0057cbd90ca55fa51c596e32406e190f3866f93e
Reviewed-on: https://go-review.googlesource.com/99815
Reviewed-by: Keith Randall <khr@golang.org>
hwcap is set in archauxv, setup_auxv no longer exists.
Change-Id: I0fc9393e0c1c45192e0eff4715e9bdd69fab2653
Reviewed-on: https://go-review.googlesource.com/99779
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Directly use rc.Close instead of wrapping it with a closure.
Change-Id: I3dc1c21ccbfe031c230b035126d5ea3bc62055c3
Reviewed-on: https://go-review.googlesource.com/99716
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#23971
Change-Id: I073f278cc058aa15a23c0ea06292c02d50a3df21
Reviewed-on: https://go-review.googlesource.com/95582
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Only RotateLeft{64,32} were tested, and just for ppc64. This CL adds
tests for RotateLeft{64,32,16,8} on arm64 and amd64/386, for the cases
where the calls are actually instrinsified.
RotateLeft tests (the last ones for math/bits functions) are deleted
from asm_test.
This CL also adds a space between the "//" and the arch name in the
comments, to uniform this file to the style used in all the other
files.
Change-Id: Ifc2a27261d70bcc294b4ec64490d8367f62d2b89
Reviewed-on: https://go-review.googlesource.com/99596
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Also grey out instants that represent events occurred outside the
task's span. Furthermore, if the unrelated instants represent user
annotation events but not for the task of the interest, skip rendering
completely.
This helps users to focus on the task-related events better.
UI screen shot:
https://gist.github.com/hyangah/1df5d2c8f429fd933c481e9636b89b55#file-golang-org_cl_99035
Change-Id: I2b5aef41584c827f8c1e915d0d8e5c95fe2b4b65
Reviewed-on: https://go-review.googlesource.com/99035
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Strangely enough, the existing implementation used adjusted (by line
directives) source positions to determine layout and thus required
position corrections when printing a line directive.
Instead, just use the unadjusted, absolute source positions and then
printing a line directive doesn't require any adjustments, only some
care to make sure it remains in column 1 as before.
The new code doesn't need to parse line directives anymore and simply
ensures that comments with the //line prefix and starting in column 1
remain in that position. That is a slight change from the old behavior
(which ignored incorrect line directives, e.g. because they had an
invalid line number) but unlikely to show up in real code.
This is prep work for handling of line directives that also specify
columns (which now won't require much special handling anymore).
For #24143.
Change-Id: I07eb2e1b35b37337e632e3dbf5b70c783c615f8a
Reviewed-on: https://go-review.googlesource.com/99621
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
'"' has special semantic meaning that conflicts with using it as Comma.
Change-Id: Ife25ba43ca25dba2ea184c1bb7579a230d376059
Reviewed-on: https://go-review.googlesource.com/99696
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's a bit mysterious that _defer.sp is a uintptr that gets
stack-adjusted explicitly while _panic.argp is an unsafe.Pointer that
doesn't, but turns out to be critically important when a deferred
function grows the stack before doing a recover.
Add a comment explaining that this works because _panic values live on
the stack. Enforce this by marking _panic go:notinheap.
Change-Id: I9ca49e84ee1f86d881552c55dccd0662b530836b
Reviewed-on: https://go-review.googlesource.com/99735
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
On all non-x86 arches, runtime.abort simply reads from nil.
Unfortunately, if this happens on a user stack, the signal handler
will dutifully turn this into a panicmem, which lets user defers run
and which user code can even recover from.
To fix this, add an explicit check to the signal handler that turns
faults in abort into hard crashes directly in the signal handler. This
has the added benefit of giving a register dump at the abort point.
Change-Id: If26a7f13790745ee3867db7f53b72d8281176d70
Reviewed-on: https://go-review.googlesource.com/93661
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Everything except for amd64, amd64p32, and 386 currently defines and
uses an abort function. This CL makes these match. The next CL will
recognize the abort function to make this more useful.
Change-Id: I7c155871ea48919a9220417df0630005b444f488
Reviewed-on: https://go-review.googlesource.com/93660
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently, throw may grow the stack, which means whenever we call it
from a context where it's not safe to grow the stack, we first have to
switch to the system stack. This is pretty easy to get wrong.
Fix this by making throw switch to the system stack so it doesn't grow
the stack and is hence safe to call without a system stack switch at
the call site.
The only thing this complicates is badsystemstack itself, which would
now go into an infinite loop before printing anything (previously it
would also go into an infinite loop, but would at least print the
error first). Fix this by making badsystemstack do a direct write and
then crash hard.
Change-Id: Ic5b4a610df265e47962dcfa341cabac03c31c049
Reviewed-on: https://go-review.googlesource.com/93659
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Currently parts of unrecoverable panic handling (notably, printing
panic messages) can happen on the user stack. This may grow the stack,
which is generally fine, but if we're handling a runtime panic, it's
better to do as little as possible in case the runtime is in an
inconsistent state.
Hence, this commit rearranges the handling of unrecoverable panics so
that it's done entirely on the system stack.
This is mostly a matter of shuffling code a bit so everything can move
into a systemstack block. The one slight subtlety is in the "panic
during panic" case, where we now depend on startpanic_m's caller to
print the stack rather than startpanic_m itself. To make this work,
startpanic_m now returns a boolean indicating that the caller should
avoid trying to print any panic messages and get right to the stack
trace. Since the caller is already in a position to do this, this
actually simplifies things a little.
Change-Id: Id72febe8c0a9fb31d9369b600a1816d65a49bfed
Reviewed-on: https://go-review.googlesource.com/93658
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The previous CL introduced isConstDelta. Use it to simplify the
OpSlicemask optimization in the prove pass. This passes toolstash
-cmp.
Change-Id: If2aa762db4cdc0cd1c581a536340530a9831081b
Reviewed-on: https://go-review.googlesource.com/87481
Reviewed-by: Keith Randall <khr@golang.org>
This adds four new deductions to the prove pass, all related to adding
or subtracting one from a value. This is the first hint of actual
arithmetic relations in the prove pass.
The most effective of these is
x-1 >= w && x > min ⇒ x > w
This helps eliminate bounds checks in code like
if x > 0 {
// do something with s[x-1]
}
Altogether, these deductions prove an additional 260 branches in std
and cmd. Furthermore, they will let us eliminate some tricky
compiler-inserted panics in the runtime that are interfering with
static analysis.
Fixes#23354.
Change-Id: I7088223e0e0cd6ff062a75c127eb4bb60e6dce02
Reviewed-on: https://go-review.googlesource.com/87480
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
This adds a few simple deductions to the prove pass' fact table to
derive unsigned concrete limits from signed concrete limits where
possible.
This tweak lets the pass prove 70 additional branch conditions in std
and cmd.
This is based on a comment from the recently-deleted factsTable.get:
"// TODO: also use signed data if lim.min >= 0".
Change-Id: Ib4340249e7733070f004a0aa31254adf5df8a392
Reviewed-on: https://go-review.googlesource.com/87479
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
Reviewed-by: Keith Randall <khr@golang.org>
Currently the prove pass uses implication queries. For each block, it
collects the set of branch conditions leading to that block, and
queries this fact table for whether any of these facts imply the
block's own branch condition (or its inverse). This works remarkably
well considering it doesn't do any deduction on these facts, but it
has various downsides:
1. It requires an implementation both of adding facts to the table and
determining implications. These are very nearly duals of each
other, but require separate implementations. Likewise, the process
of asserting facts of dominating branch conditions is very nearly
the dual of the process of querying implied branch conditions.
2. It leads to less effective use of derived facts. For example, the
prove pass currently derives facts about the relations between len
and cap, but can't make use of these unless a branch condition is
in the exact form of a derived fact. If one of these derived facts
contradicts another fact, it won't notice or make use of this.
This CL changes the approach of the prove pass to instead use
*contradiction* instead of implication. Rather than ever querying a
branch condition, it simply adds branch conditions to the fact table.
If this leads to a contradiction (specifically, it makes the fact set
unsatisfiable), that branch is impossible and can be cut. As a result,
1. We can eliminate the code for determining implications
(factsTable.get disappears entirely). Also, there is now a single
implementation of visiting and asserting branch conditions, since
we don't have to flip them around to treat them as facts in one
place and queries in another.
2. Derived facts can be used effectively. It doesn't matter *why* the
fact table is unsatisfiable; a contradiction in any of the facts is
enough.
3. As an added benefit, it's now quite easy to avoid traversing beyond
provably-unreachable blocks. In contrast, the current
implementation always visits all blocks.
The prove pass already has nearly all of the mechanism necessary to
compute unsatisfiability, which means this both simplifies the code
and makes it more powerful.
The only complication is that the current implication procedure has a
hack for dealing with the 0 <= Args[0] condition of OpIsInBounds and
OpIsSliceInBounds. We replace this with asserting the appropriate fact
when we process one of these conditions. This seems much cleaner
anyway, and works because we can now take advantage of derived facts.
This has no measurable effect on compiler performance.
Effectiveness:
There is exactly one condition in all of std and cmd that this fails
to prove that the old implementation could: (int64(^uint(0)>>1) < x)
in encoding/gob. This can never be true because x is an int, and it's
basically coincidence that the old code gets this. (For example, it
fails to prove the similar (x < ^int64(^uint(0)>>1)) condition that
immediately precedes it, and even though the conditions are logically
unrelated, it wouldn't get the second one if it hadn't first processed
the first!)
It does, however, prove a few dozen additional branches. These come
from facts that are added to the fact table about the relations
between len and cap. These were almost never queried directly before,
but could lead to contradictions, which the unsat-based approach is
able to use.
There are exactly two branches in std and cmd that this implementation
proves in the *other* direction. This sounds scary, but is okay
because both occur in already-unreachable blocks, so it doesn't matter
what we chose. Because the fact table logic is sound but incomplete,
it fails to prove that the block isn't reachable, even though it is
able to prove that both outgoing branches are impossible. We could
turn these blocks into BlockExit blocks, but it doesn't seem worth the
trouble of the extra proof effort for something that happens twice in
all of std and cmd.
Tests:
This CL updates test/prove.go to change the expected messages because
it can no longer give a "reason" why it proved or disproved a
condition. It also adds a new test of a branch it couldn't prove
before.
It mostly guts test/sliceopt.go, removing everything related to slice
bounds optimizations and moving a few relevant tests to test/prove.go.
Much of this test is actually unreachable. The new prove pass figures
this out and doesn't try to prove anything about the unreachable
parts. The output on the unreachable parts is already suspect because
anything can be proved at that point, so it's really just a regression
test for an algorithm the compiler no longer uses.
This is a step toward fixing #23354. That issue is quite easy to fix
once we can use derived facts effectively.
Change-Id: Ia48a1b9ee081310579fe474e4a61857424ff8ce8
Reviewed-on: https://go-review.googlesource.com/87478
Reviewed-by: Keith Randall <khr@golang.org>
This replaces the open-coded intersection of limits in the prove pass
with a general limit intersection operation. This should get identical
results except in one case where it's more precise: when handling an
equality relation, if the value is *outside* the existing range, this
will reduce the range to empty rather than resetting it. This will be
important to a follow-up CL where we can take advantage of empty
ranges.
For #23354.
Change-Id: I3d3d75924f61b1da1cb604b3a9d189b26fb3a14e
Reviewed-on: https://go-review.googlesource.com/87477
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Alexandru Moșoi <alexandru@mosoi.ro>
The Newton sqrtInverse procedure we use to compute Float.Sqrt should
not allocate a number of times proportional to the number of Newton
iterations we need to reach the desired precision.
At the beginning the function the target precision is known, so even
if we do want to perform the early steps at low precisions (to save
time), it's still possible to pre-allocate larger backing arrays, both
for the temp variables in the loop and the variable that'll hold the
final result.
There's one complication. At the following line:
u.Sub(three, u)
the Sub method will allocate, because the receiver aliases one of the
arguments, and the large backing array we initially allocated for u
will be replaced by a smaller one allocated by Sub. We can work around
this by introducing a second temp variable u2 that we use to hold the
Sub call result.
Overall, the sqrtInverse procedure still allocates a number of times
proportional to the number of Newton steps, because unfortunately a
few of the Mul calls in the Newton function allocate; but at least we
allocate less in the function itself.
FloatSqrt/256-4 1.97µs ± 1% 1.84µs ± 1% -6.61% (p=0.000 n=8+8)
FloatSqrt/1000-4 4.80µs ± 3% 4.28µs ± 1% -10.78% (p=0.000 n=8+8)
FloatSqrt/10000-4 40.0µs ± 1% 38.3µs ± 1% -4.15% (p=0.000 n=8+8)
FloatSqrt/100000-4 955µs ± 1% 932µs ± 0% -2.49% (p=0.000 n=8+7)
FloatSqrt/1000000-4 79.8ms ± 1% 79.4ms ± 1% ~ (p=0.105 n=8+8)
name old alloc/op new alloc/op delta
FloatSqrt/256-4 816B ± 0% 512B ± 0% -37.25% (p=0.000 n=8+8)
FloatSqrt/1000-4 2.50kB ± 0% 1.47kB ± 0% -41.03% (p=0.000 n=8+8)
FloatSqrt/10000-4 23.5kB ± 0% 18.2kB ± 0% -22.62% (p=0.000 n=8+8)
FloatSqrt/100000-4 251kB ± 0% 173kB ± 0% -31.26% (p=0.000 n=8+8)
FloatSqrt/1000000-4 4.61MB ± 0% 2.86MB ± 0% -37.90% (p=0.000 n=8+8)
name old allocs/op new allocs/op delta
FloatSqrt/256-4 12.0 ± 0% 8.0 ± 0% -33.33% (p=0.000 n=8+8)
FloatSqrt/1000-4 19.0 ± 0% 9.0 ± 0% -52.63% (p=0.000 n=8+8)
FloatSqrt/10000-4 35.0 ± 0% 14.0 ± 0% -60.00% (p=0.000 n=8+8)
FloatSqrt/100000-4 55.0 ± 0% 23.0 ± 0% -58.18% (p=0.000 n=8+8)
FloatSqrt/1000000-4 122 ± 0% 75 ± 0% -38.52% (p=0.000 n=8+8)
Change-Id: I950dbf61a40267a6cca82ae72524c3024bcb149c
Reviewed-on: https://go-review.googlesource.com/87659
Reviewed-by: Robert Griesemer <gri@golang.org>
Instead of creating a new &nodfp expression for every recover() call,
or a new nodpc variable for every function instrumented by the race
detector, this CL introduces two new uintptr-typed pseudo-variables
callerSP and callerPC. These pseudo-variables act just like calls to
the runtime's getcallersp() and getcallerpc() functions.
For consistency, change runtime.gorecover's builtin stub's parameter
type from "*int32" to "uintptr".
Passes toolstash-check, but toolstash-check -race fails because of
register allocator changes.
Change-Id: I985d644653de2dac8b7b03a28829ad04dfd4f358
Reviewed-on: https://go-review.googlesource.com/99416
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
All calls to walkstmt/walkexpr/etc should be rooted from funccompile,
whereas transformclosure and fninit are called by main.
Passes toolstash-check.
Change-Id: Ic880e2d2d83af09618ce4daa8e7716f6b389e53e
Reviewed-on: https://go-review.googlesource.com/99418
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
And delete them from the asm_test.go file.
Change-Id: I124c8c352299646ec7db0968cdb0fe59a3b5d83d
Reviewed-on: https://go-review.googlesource.com/99475
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
When instructions add, and, or, xor, and movd have
constant operands in some cases more instructions are
generated than necessary by the assembler.
This adds more opcode/operand combinations to the optab
and improves the code generation for the cases where the
size and sign of the constant allows the use of 1
instructions instead of 2.
Example of previous code:
oris r3, r0, 0
ori r3, r3, 65533
now:
ori r3, r0, 65533
This does not significantly reduce the overall binary size
because the improvement depends on the constant value.
Some procedures show a 1-2% reduction in size. This improvement
could also be significant in cases where the extra instructions
occur in a critical loop.
Testcase ppc64enc.s was added to cmd/asm/internal/asm/testdata
with the variations affected by this change.
Updates #23845
Change-Id: I7fdf2320c95815d99f2755ba77d0c6921cd7fad7
Reviewed-on: https://go-review.googlesource.com/95135
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
In the situation where a quoted field is necessary, avoid processing
each UTF-8 rune one-by-one, which causes mangling of invalid sequences
into utf8.RuneError, causing a loss of information.
Instead, search only for the escaped characters, handle those specially
and copy everything else in between verbatim.
This symmetrically matches the behavior of Reader.
Fixes#24298
Change-Id: I9276f64891084ce8487678f663fad711b4095dbb
Reviewed-on: https://go-review.googlesource.com/99297
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This was already done for normal parameters, and the same logic
applies for receiver parameters too.
Updates #24305.
Change-Id: Ia2a46f68d14e8fb62004ff0da1db0f065a95a1b7
Reviewed-on: https://go-review.googlesource.com/99335
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Without the change to cover.go, the new test fails with
panic: overlapping edits: [4946,4950)->"", [4947,4947)->"thisNameMustBeVeryLongToCauseOverflowOfCounterIncrementStatementOntoNextLineForTest.Count[112]++;"
The original code inserts "else{", deletes "else", and then positions
a new block just after the "}" that must come before the "else".
That works on gofmt'ed code, but fails if the code looks like "}else".
When there is no space between the "{" and the "else", the new block
is inserted into a location that we are deleting, leading to the
"overlapping edits" mentioned above.
This CL fixes this case by not deleting the "else" but just using the
one that is already there. That requires adjust the block offset to
come after the "{" that we insert.
Fixes#23927
Change-Id: I40ef592490878765bbce6550ddb439e43ac525b2
Reviewed-on: https://go-review.googlesource.com/98935
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Currently if a profiling signal arrives while executing within a VDSO
the profiler will report _ExternalCode, which is needlessly confusing
for a pure Go program. Change the VDSO calling code to record the
caller's PC/SP, so that we can do a traceback from that point. If that
fails for some reason, report _VDSO rather than _ExternalCode, which
should at least point in the right direction.
This adds some instructions to the code that calls the VDSO, but the
slowdown is reasonably negligible:
name old time/op new time/op delta
ClockVDSOAndFallbackPaths/vDSO-8 40.5ns ± 2% 41.3ns ± 1% +1.85% (p=0.002 n=10+10)
ClockVDSOAndFallbackPaths/Fallback-8 41.9ns ± 1% 43.5ns ± 1% +3.84% (p=0.000 n=9+9)
TimeNow-8 41.5ns ± 3% 41.5ns ± 2% ~ (p=0.723 n=10+10)
Fixes#24142
Change-Id: Iacd935db3c4c782150b3809aaa675a71799b1c9c
Reviewed-on: https://go-review.googlesource.com/97315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This normalizes the Linux code to act like other targets. The size
argument to the rt_sigaction system call is pushed to a single
function, sysSigaction.
This is intended as a simplification step for CL 93875 for #14327.
Change-Id: I594788e235f0da20e16e8a028e27ac8c883907c4
Reviewed-on: https://go-review.googlesource.com/99077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
CL 99175 added TestVetWithOnlyCgoFiles. However, this
test is failing on platforms where cgo is disabled,
because no file can be built.
This change fixes TestVetWithOnlyCgoFiles by skipping
this test when cgo is disabled.
Fixes#24304.
Change-Id: Ibb38fcd3e0ed1a791782145d3f2866f12117c6fe
Reviewed-on: https://go-review.googlesource.com/99275
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
While working on standalone builds of gomobile bindings, I ran into
errors on the form:
gcc_darwin_arm.c:30:31: error: ambiguous expansion of macro 'nil' [-Werror,-Wambiguous-macro]
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS11.2.sdk/usr/include/MacTypes.h:94:15: note: expanding this definition of 'nil'
Fix it by undefining nil before defining it in libcgo.h.
Change-Id: I8e9660a68c6c351e592684d03d529f0d182c0493
Reviewed-on: https://go-review.googlesource.com/99215
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CgoFiles is not included in GoFiles, so we need to check both.
Fixes#24193
Change-Id: I6a67bd912e3d9a4be0eae8fa8db6fa8a07fb5df3
Reviewed-on: https://go-review.googlesource.com/99175
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We already require expressions to have already been typechecked before
reaching walk. Moreover, all untyped expressions should have been
converted to their default type by walk.
However, in practice, we've been somewhat sloppy and inconsistent
about ensuring this. In particular, a lot of AST rewrites ended up
leaving untyped bool expressions scattered around. These likely aren't
harmful in practice, but it seems worth cleaning up.
The two most common cases addressed by this CL are:
1) When generating OIF and OFOR nodes, we would often typecheck the
conditional expression, but not apply defaultlit to force it to the
expression's default type.
2) When rewriting string comparisons into more fundamental primitives,
we were simply overwriting r.Type with the desired type, which didn't
propagate the type to nested subexpressions. These are fixed by
utilizing finishcompare, which correctly handles this (and is already
used by other comparison lowering rewrites).
Lastly, walkexpr is extended to assert that it's not called on untyped
expressions.
Fixes#23834.
Change-Id: Icbd29648a293555e4015d3b06a95a24ccbd3f790
Reviewed-on: https://go-review.googlesource.com/98337
Reviewed-by: Robert Griesemer <gri@golang.org>
Build could use the package comment from test files to populate the .Doc
field on *Package.
As go list uses this data and several packages in the standard library
have tests with package comments, this lead to:
$ go list -f '{{.Doc}}' flag container/heap image
These examples demonstrate more intricate uses of the flag package.
This example demonstrates an integer heap built using the heap interface.
This example demonstrates decoding a JPEG image and examining its pixels.
This change now only examines non-test files when attempting to populate
.Doc, resulting in the expected behavior:
$ gotip list -f '{{.Doc}}' flag container/heap image
Package flag implements command-line flag parsing.
Package heap provides heap operations for any type that implements heap.Interface.
Package image implements a basic 2-D image library.
Fixes#23594
Change-Id: I37171c26ec5cc573efd273556a05223c6f675968
Reviewed-on: https://go-review.googlesource.com/96976
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
They do not match the file name patterns of
*_GOOS
*_GOARCH
*_GOOS_GOARCH
therefore the implicit linux constraint was not being added.
Change-Id: Ie506c51cee6818db445516f96fffaa351df62cf5
Reviewed-on: https://go-review.googlesource.com/99116
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The host GOARCH is most likely supported (386, amd64, arm, arm64).
Change-Id: I86324b9c00f22c592ba54bda7d2ae97c86bda904
Reviewed-on: https://go-review.googlesource.com/99155
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
os.Stat implementation uses instructions described at
https://blogs.msdn.microsoft.com/oldnewthing/20100212-00/?p=14963/
to distinguish symlinks. In particular, it calls
GetFileAttributesEx or FindFirstFile and checks
either WIN32_FILE_ATTRIBUTE_DATA.dwFileAttributes
or WIN32_FIND_DATA.dwFileAttributes to see if
FILE_ATTRIBUTES_REPARSE_POINT flag is set.
And that seems to worked fine so far.
But now we discovered that OneDrive root folder
is determined as directory:
c:\>dir C:\Users\Alex | grep OneDrive
30/11/2017 07:25 PM <DIR> OneDrive
c:\>
while Go identified it as symlink.
But we did not follow Microsoft's advice to the letter - we never
checked WIN32_FIND_DATA.Reserved0. And adding that extra check
makes Go treat OneDrive as symlink. So use FindFirstFile and
WIN32_FIND_DATA.Reserved0 to determine symlinks.
Fixes#22579
Change-Id: I0cb88929eb8b47b1d24efaf1907ad5a0e20de83f
Reviewed-on: https://go-review.googlesource.com/86556
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@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>
Bring these functions next to each other, and clean them up a little
bit. Also, change emitptrargsmap to take Curfn as a parameter instead
of a global.
Passes toolstash-check.
Change-Id: Ib9c94fda3b2cb6f0dcec1585622b33b4f311b5e9
Reviewed-on: https://go-review.googlesource.com/99075
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, for slow map key types (i.e., any type other than a 32-bit
or 64-bit plain memory type), we would rewrite
defer delete(m, k)
into
ktmp := k
defer delete(m, &ktmp)
However, if the defer statement was inside a loop, we would end up
reusing the same ktmp value for all of the deferred deletes.
We already rewrite
defer print(x, y, z)
into
defer func(a1, a2, a3) {
print(a1, a2, a3)
}(x, y, z)
This CL generalizes this rewrite to also apply for slow map deletes.
This could be extended to apply even more generally to other builtins,
but as discussed on #24259, there are cases where we must *not* do
this (e.g., "defer recover()"). However, if we elect to do this more
generally, this CL should still make that easier.
Lastly, while here, fix a few isues in wrapCall (nee walkprintfunc):
1) lookupN appends the generation number to the symbol anyway, so "%d"
was being literally included in the generated function names.
2) walkstmt will be called when the function is compiled later anyway,
so no need to do it now.
Fixes#24259.
Change-Id: I70286867c64c69c18e9552f69e3f4154a0fc8b04
Reviewed-on: https://go-review.googlesource.com/99017
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
And remove them from ssa_test.
Change-Id: If767af662801219774d1bdb787c77edfa6067770
Reviewed-on: https://go-review.googlesource.com/98976
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Current implementation doesn't consider MOVDreg type operand and fail to combine
it into larger store. This patch fixes the issue.
Fixes#24242
Change-Id: I7d68697f80e76f48c3528ece01a602bf513248ec
Reviewed-on: https://go-review.googlesource.com/98397
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
While running make.bash, over 5% of all pointer writes
come from encoding/binary doing struct reads.
This change replaces slicing during such reads with an offset.
This avoids updating the slice pointer with every
struct field read or write.
This has no impact when the write barrier is off.
Running the benchmarks with GOGC=1, however,
shows significant improvement:
name old time/op new time/op delta
ReadStruct-8 13.2µs ± 6% 10.1µs ± 5% -23.24% (p=0.000 n=10+10)
name old speed new speed delta
ReadStruct-8 5.69MB/s ± 6% 7.40MB/s ± 5% +30.18% (p=0.000 n=10+10)
Change-Id: I22904263196bfeddc38abe8989428e263aee5253
Reviewed-on: https://go-review.googlesource.com/98757
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
And remove them from ssa_test.
Change-Id: I3efac5fea529bb0efa2dae32124530482ba5058e
Reviewed-on: https://go-review.googlesource.com/98815
Reviewed-by: Keith Randall <khr@golang.org>
Instead, mirror androidtest.bash and build once, then run run.bash.
Change-Id: I174ae30b2a429a62b20bb290a70cb07ed712b1e4
Reviewed-on: https://go-review.googlesource.com/98915
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Auto-detecting GOARM on Android makes as little sense as for nacl/arm
and darwin/arm.
Also update androidtest.sh to not require GOARM set.
Change-Id: Id409ce1573d3c668d00fa4b7e3562ad7ece6fef5
Reviewed-on: https://go-review.googlesource.com/98875
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 98095 got the check wrong. We should be testing
'getg() == getg().m.curg', not 'getg().m == getg().m.curg'.
Change-Id: I32f6238b00409b67afa8efe732513d542aec5bc7
Reviewed-on: https://go-review.googlesource.com/98855
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
