'%SystemRoot%/System32/chcp.com' is a tool on Windows that
is used to change the active code page in the console.
'go test path/filepath' can fail with:
"'chcp' is not recognized as an internal or external command"
The test uses a custom PATH variable but does not include
'%SystemRoot%/System32'. Always append that to PATH.
Updates #24709
Change-Id: Ib4c83ffdcc5dd6eb7bb34c07386cf2ab61dcae57
GitHub-Last-Rev: fac92613cc
GitHub-Pull-Request: golang/go#25089
Reviewed-on: https://go-review.googlesource.com/109362
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On amd64, Ctz must include special handling of zeros.
But the prove pass has enough information to detect whether the input
is non-zero, allowing a more efficient lowering.
Introduce new CtzNonZero ops to capture and use this information.
Benchmark code:
func BenchmarkVisitBits(b *testing.B) {
b.Run("8", func(b *testing.B) {
for i := 0; i < b.N; i++ {
x := uint8(0xff)
for x != 0 {
sink = bits.TrailingZeros8(x)
x &= x - 1
}
}
})
// and similarly so for 16, 32, 64
}
name old time/op new time/op delta
VisitBits/8-8 7.27ns ± 4% 5.58ns ± 4% -23.35% (p=0.000 n=28+26)
VisitBits/16-8 14.7ns ± 7% 10.5ns ± 4% -28.43% (p=0.000 n=30+28)
VisitBits/32-8 27.6ns ± 8% 19.3ns ± 3% -30.14% (p=0.000 n=30+26)
VisitBits/64-8 44.0ns ±11% 38.0ns ± 5% -13.48% (p=0.000 n=30+30)
Fixes#25077
Change-Id: Ie6e5bd86baf39ee8a4ca7cadcf56d934e047f957
Reviewed-on: https://go-review.googlesource.com/109358
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Running 'go run *.go' in the gen directory resulted in this diff.
Change-Id: Iee398a720f54d3f2c3c122fc6fc45a708a39e45e
Reviewed-on: https://go-review.googlesource.com/109575
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When creating a directory, Writer.Create now returns a dummy
io.Writer that always returns an error on Write.
Fixes#24043
Change-Id: I7792f54440d45d22d0aa174cba5015ed5fab1c5c
Reviewed-on: https://go-review.googlesource.com/108615
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
NewReplacer's documentation says that "replacements are performed in
order", meaning that substrings are replaced in the order they appear
in the target string, and not that the old->new replacements are
applied in the order they're passed to NewReplacer.
Rephrase the doc to make this clearer.
Fixes#25071
Change-Id: Icf3aa6a9d459b94764c9d577e4a76ad8c04d158d
Reviewed-on: https://go-review.googlesource.com/109375
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Doesn't cause an error, see #25085.
But we should fix it nonetheless.
Change-Id: I7b6799e0a95475202cacefc3a7f02487e61bfd31
Reviewed-on: https://go-review.googlesource.com/109355
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change implements math.Round as an intrinsic on ppc64x so it can be
done using a single instruction.
benchmark old ns/op new ns/op delta
BenchmarkRound-16 2.60 0.69 -73.46%
Change-Id: I9408363e96201abdfc73ced7bcd5f0c29db006a8
Reviewed-on: https://go-review.googlesource.com/109395
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>
When we moved racewalk to buildssa, we disabled SSA of structs with
2-4 fields while instrumenting because it caused false dependencies
because for "x.f" we would emit
(StructSelect (Load (Addr x)) "f")
Even though we later simplify this to
(Load (OffPtr (Addr x) "f"))
the instrumentation saw a load of x in its entirety and would issue
appropriate race/msan calls.
The fix taken here is to directly emit the OffPtr form when x.f is
addressable and can't be represented in SSA form.
Change-Id: I0caf37bced52e9c16937466b0ac8cab6d356e525
Reviewed-on: https://go-review.googlesource.com/109360
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This is a followup for CL 93156.
Fixes#22942.
Change-Id: Ic6e2de44011d041b91454353a6f2e3b0cf590060
Reviewed-on: https://go-review.googlesource.com/108095
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that #18395 is fixed, let's see if we can insist
on vet during go test being able to type-check
packages again.
Change-Id: Iaa55a4d9c582ba743df2347d28c24f130e16e406
Reviewed-on: https://go-review.googlesource.com/108555
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Now that we can tell when a package is a split-off copy
for testing, show that in the build failures.
For example, instead of:
# regexp/syntax
../../regexp/syntax/parse.go:9:2: can't find import: "strings"
# path/filepath
../../path/filepath/match.go:12:2: can't find import: "strings"
# flag
../../flag/flag.go:75:2: can't find import: "strings"
we now print
# regexp/syntax [strings.test]
../../regexp/syntax/parse.go:9:2: can't find import: "strings"
# path/filepath [strings.test]
../../path/filepath/match.go:12:2: can't find import: "strings"
# flag [strings.test]
../../flag/flag.go:75:2: can't find import: "strings"
which gives more of a hint about what is wrong.
This is especially helpful if a package is being built multiple times,
since it explains why an error might appear multiple times:
$ go test regexp encoding/json
# regexp
../../regexp/exec.go:12:9: undefined: x
# regexp [regexp.test]
../../regexp/exec.go:12:9: undefined: x
FAIL regexp [build failed]
FAIL encoding/json [build failed]
$
Change-Id: Ie325796f6c3cf0e23f306066be8e65a30cb6b939
Reviewed-on: https://go-review.googlesource.com/108155
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Tools should be able to ask cmd/go about the dependency
graph for test binaries instead of reinventing it themselves.
Allow them to do so, with the new list -test flag.
This also fixes and tests for a bug introduced in CL 104315
that was not properly splitting dependencies on the path
between package main and the package being tested.
Change-Id: I29eb454c82893f5ee70252aaaecd9fa376eaf3c8
Reviewed-on: https://go-review.googlesource.com/107916
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The previous change sped up the pure computation form of LeadingZeros8.
This places it somewhat close to the table lookup form.
Depending on something that varies from toolchain to toolchain
(alignment, perhaps?), the slowdown from ditching the table lookup
is either 20% or 5%.
This benchmark is the best case scenario for the table lookup:
It is in the L1 cache already.
I think we're close enough that we can switch to the computational version,
and trust that the memory effects and binary size savings will be worth it.
Code:
func f8(x uint8) { z = bits.LeadingZeros8(x) }
Before:
"".f8 STEXT nosplit size=34 args=0x8 locals=0x0
0x0000 00000 (x.go:7) TEXT "".f8(SB), NOSPLIT, $0-8
0x0000 00000 (x.go:7) FUNCDATA $0, gclocals·2a5305abe05176240e61b8620e19a815(SB)
0x0000 00000 (x.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0000 00000 (x.go:7) MOVBLZX "".x+8(SP), AX
0x0005 00005 (x.go:7) MOVBLZX AL, AX
0x0008 00008 (x.go:7) LEAQ math/bits.len8tab(SB), CX
0x000f 00015 (x.go:7) MOVBLZX (CX)(AX*1), AX
0x0013 00019 (x.go:7) ADDQ $-8, AX
0x0017 00023 (x.go:7) NEGQ AX
0x001a 00026 (x.go:7) MOVQ AX, "".z(SB)
0x0021 00033 (x.go:7) RET
After:
"".f8 STEXT nosplit size=30 args=0x8 locals=0x0
0x0000 00000 (x.go:7) TEXT "".f8(SB), NOSPLIT, $0-8
0x0000 00000 (x.go:7) FUNCDATA $0, gclocals·2a5305abe05176240e61b8620e19a815(SB)
0x0000 00000 (x.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0000 00000 (x.go:7) MOVBLZX "".x+8(SP), AX
0x0005 00005 (x.go:7) MOVBLZX AL, AX
0x0008 00008 (x.go:7) LEAL 1(AX)(AX*1), AX
0x000c 00012 (x.go:7) BSRL AX, AX
0x000f 00015 (x.go:7) ADDQ $-8, AX
0x0013 00019 (x.go:7) NEGQ AX
0x0016 00022 (x.go:7) MOVQ AX, "".z(SB)
0x001d 00029 (x.go:7) RET
Change-Id: Icc7db50a7820fb9a3da8a816d6b6940d7f8e193e
Reviewed-on: https://go-review.googlesource.com/108942
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This gives an easy way to query properties of all the deps
of a set of packages, in a single go list invocation.
Go list has already done the hard work of loading these
packages, so exposing them is more efficient than
requiring a second invocation.
This will be helpful for tools asking cmd/go about build
information.
Change-Id: I90798e386246b24aad92dd13cb9e3788c7d30e91
Reviewed-on: https://go-review.googlesource.com/107776
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The test has been flaky, probably due to EAGAIN, but let's find out
for sure.
Updates #25078
Change-Id: I5a5b14bfc52cb43f25f07ca7d207b61ae9d4f944
Reviewed-on: https://go-review.googlesource.com/109359
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If X depends on Y and X was installed but Y is only present in the cache
(as happens when you "go install X") then we should report X as up-to-date,
not as stale.
This applies whether X is a package or a main binary.
Fixes#24558.
Fixes#23818.
Change-Id: I26a0b375b1f7f7ac909cc0db68e92f4e04529208
Reviewed-on: https://go-review.googlesource.com/107957
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Now that vet always has type information, there's no reason to use
string handling on type names to gather information about them, such as
whether or not they are a local type.
The semantics remain the same - the only difference should be that the
implementation is less fragile and simpler.
Change-Id: I71386b4196922e4c9f2653d90abc382efbf01b3c
Reviewed-on: https://go-review.googlesource.com/95915
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
The file name suffix arm64 already limits the file to be build only on arm64.
Change-Id: I33db713041b6dec9eb00889bac3b54c727e90743
Reviewed-on: https://go-review.googlesource.com/108986
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The test wants to check that copies of Cond are detected at runtime.
Make a copy that isn't detected by vet at compile time.
Change-Id: I933ab1003585f75ba96723563107f1ba8126cb72
Reviewed-on: https://go-review.googlesource.com/108557
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The existing text makes it seem like there's no way
to use GitHub over HTTPS. There is. Explain that.
Also, the existing text suggests explicit checkout into $GOPATH,
which is not going to work in the new module world.
Drop that alternative.
Also, the existing text uses pushInsteadOf instead of insteadOf,
which would have the effect of being able to push to a private
repo but not clone it in the first place. That seems not helpful,
so suggest insteadOf instead.
Fixes#18927.
Change-Id: Ic358b66f88064b53067d174a2a1591ac8bf96c88
Reviewed-on: https://go-review.googlesource.com/107775
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, 's' was only written to, never read,
which is disallowed by the spec. cmd/compile
has a bug where it doesn't notice this when a
closure is involved, but go/types does notice,
which was making "go vet" fail.
This CL moves the variable into the closure
and also makes sure to use it.
Change-Id: I2d83fb6b5c1c9018df03533e966cbdf455f83bf9
Reviewed-on: https://go-review.googlesource.com/108556
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We were using absolute paths in the #line directives in the export
header file. This makes the header file change if you move GOPATH.
The absolute paths aren't helpful for the final user, which is some C
program elsewhere.
Fixes#24945
Change-Id: I2da32c9b477df578bd5087435a03fe97abe462e3
Reviewed-on: https://go-review.googlesource.com/108315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Get rid of a bunch of stuff we've already done.
Change-Id: Ibae4be7535ddb58590a072a2390c5f3e948c2fd7
Reviewed-on: https://go-review.googlesource.com/109136
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reduces the API surface of Type slightly (for #25056), but also
makes it more consistent with the reflect and go/types APIs.
Passes toolstash-check.
Change-Id: Ief9a8eb461ae6e88895f347e2a1b7b8a62423222
Reviewed-on: https://go-review.googlesource.com/109138
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was an artifact from when we had a separate ssa.Type interface to
break circular dependency between packages ssa and gc. It's no longer
needed now that package ssa directly uses package types.
Change-Id: I6a93e5d79082815f7f0eb89507381969cc6cb403
Reviewed-on: https://go-review.googlesource.com/109137
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
A task may have other user annotation events after the task ends.
So far, task.lastTimestamp returned the task end event if the
event available. This change introduces task.endTimestamp for that
and makes task.lastTimestamp returns the "last" seen event's timestamp
if the task is ended.
If the task is not ended, both returns the last timestamp of the entire
trace assuming the task is still active.
This fixes the task-oriented trace view mode not to drop user
annotation instances when they appear outside a task's lifespan.
Adds a test.
Change-Id: Iba1062914f224edd521b9ee55c6cd5e180e55359
Reviewed-on: https://go-review.googlesource.com/109175
Reviewed-by: Heschi Kreinick <heschi@google.com>
For ppc64, skip -linkmode=external per
https://go-review.googlesource.com/c/go/+/106775#message-f95b9bd716e3d9ebb3f47a50492cde9f2972e859
For Solaris, apparently type.* isn't the same as runtime.types. I don't
know why, but runtime.types is what goes into moduledata, and so it's
definitely the more correct thing to use.
Fixes: #24983
Change-Id: I6b465ac7b8f91ce55a63acbd7fe76e4a2dbb6f22
Reviewed-on: https://go-review.googlesource.com/108955
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, avoid Region creation when tracing is disabled.
Unfortunate side-effect of this change is that we no longer trace
pre-existing regions in tracing, but we can add the feature in
the future when we find it useful and justifiable. Until then,
let's avoid the overhead from this low-level api use as much as
possible.
goos: linux
goarch: amd64
pkg: runtime/trace
// Trace disabled
BenchmarkStartRegion-12 2000000000 0.66 ns/op 0 B/op 0 allocs/op
BenchmarkNewTask-12 30000000 40.4 ns/op 56 B/op 2 allocs/op
// Trace enabled, -trace=/dev/null
BenchmarkStartRegion-12 5000000 287 ns/op 32 B/op 1 allocs/op
BenchmarkNewTask-12 5000000 283 ns/op 56 B/op 2 allocs/op
Also, skip other tests if tracing is already enabled.
Change-Id: Id3028d60b5642fcab4b09a74fd7d79361a3861e5
Reviewed-on: https://go-review.googlesource.com/109115
Reviewed-by: Peter Weinberger <pjw@google.com>
"Span" is a commonly used term in many distributed tracing systems
(Dapper, OpenCensus, OpenTracing, ...). They use it to refer to a
period of time, not necessarily tied into execution of underlying
processor, thread, or goroutine, unlike the "Span" of runtime/trace
package.
Since distributed tracing and go runtime execution tracing are
already similar enough to cause confusion, this CL attempts to avoid
using the same word if possible.
"Region" is being used in a certain tracing system to refer to a code
region which is pretty close to what runtime/trace.Span currently
refers to. So, replace that.
https://software.intel.com/en-us/itc-user-and-reference-guide-defining-and-recording-functions-or-regions
This CL also tweaks APIs a bit based on jbd and heschi's comments:
NewContext -> NewTask
and it now returns a Task object that exports End method.
StartSpan -> StartRegion
and it now returns a Region object that exports End method.
Also, changed WithSpan to WithRegion and it now takes func() with no
context. Another thought is to get rid of WithRegion. It is a nice
concept but in practice, it seems problematic (a lot of code churn,
and polluting stack trace). Already, the tracing concept is very low
level, and we hope this API to be used with great care.
Recommended usage will be
defer trace.StartRegion(ctx, "someRegion").End()
Left old APIs untouched in this CL. Once the usage of them are cleaned
up, they will be removed in a separate CL.
Change-Id: I73880635e437f3aad51314331a035dd1459b9f3a
Reviewed-on: https://go-review.googlesource.com/108296
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: JBD <jbd@google.com>
We currently rewrite
(TESTQ (MOVQconst [c] x)) into (TESTQconst [c] x)
and (TESTQconst [-1] x) into (TESTQ x x)
if x is a (MOVQconst [-1]) we will be stuck in the endless rewrite loop.
Don't perform the rewrite in such cases.
Fixes#25006
Change-Id: I77f561ba2605fc104f1e5d5c57f32e9d67a2c000
Reviewed-on: https://go-review.googlesource.com/108879
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The Go's heap profile contains four kinds of samples
(inuse_space, inuse_objects, alloc_space, and alloc_objects).
The pprof tool by default chooses the inuse_space (the bytes
of live, in-use objects). When analyzing the current memory
usage the choice of inuse_space as the default may be useful,
but in some cases, users are more interested in analyzing the
total allocation statistics throughout the program execution.
For example, when we analyze the memory profile from benchmark
or program test run, we are more likely interested in the whole
allocation history than the live heap snapshot at the end of
the test or benchmark.
The pprof tool provides flags to control which sample type
to be used for analysis. However, it is one of the less-known
features of pprof and we believe it's better to choose the
right type of samples as the default when producing the profile.
This CL introduces a new type of profile, "allocs", which is
the same as the "heap" profile but marks the alloc_space
as the default type unlike heap profiles that use inuse_space
as the default type.
'go test -memprofile=...' command is changed to use the new
"allocs" profile type instead of the traditional "heap" profile.
Fixes#24443
Change-Id: I012dd4b6dcacd45644d7345509936b8380b6fbd9
Reviewed-on: https://go-review.googlesource.com/102696
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Memory arguments for debug/control register moves are a
minefield for programmer: not useful, but can lead to errors.
See referenced issue for detailed explanation.
Fixes#24981
Change-Id: I918e81cd4a8b1dfcfc9023cdfc3de45abe29e749
Reviewed-on: https://go-review.googlesource.com/107075
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
gc/ssa.go initilizes SP and SB values with TUINTPTR type.
Assign same type in SSA tests and modify check.go to catch
mismatching types for those ops.
This makes SSA tests more consistent.
Change-Id: I798440d57d00fb949d1a0cd796759c9b82a934bd
Reviewed-on: https://go-review.googlesource.com/106658
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
