summaryrefslogtreecommitdiff
path: root/src/cmd
Commit message (Collapse)AuthorAgeFilesLines
* cmd/asm: remove unsupported opcodes MOVNP and STLP for arm64HEADmastererifan012023-05-183-468/+442
| | | | | | | | | | | | | ARM64 doesn't have MOVNP/MOVNPW and STLP/STLPW instructions, which are currently useless instructions as well. This CL deletes them. At the same time this CL sorts the opcodes by name, which looks cleaner. Change-Id: I25cfb636b23356ba0a50cba527a8c85b3f7e2ee4 Reviewed-on: https://go-review.googlesource.com/c/go/+/495695 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Eric Fang <eric.fang@arm.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/compile: enable more lenient type inference for untyped argumentsRobert Griesemer2023-05-182-4/+6
| | | | | | | | | | | | | | | | | This enables the implementation for proposal #58671, which is a likely accept. By enabling it early we get a bit extra soak time for this feature. The change can be reverted trivially, if need be. For #58671. Change-Id: Id6c27515e45ff79f4f1d2fc1706f3f672ccdd1ab Reviewed-on: https://go-review.googlesource.com/c/go/+/495955 Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* runtime/cgo: store M for C-created thread in pthread keyCherry Mui2023-05-179-3/+210
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reapplies CL 485500, with a fix drafted in CL 492987 incorporated. CL 485500 is reverted due to #60004 and #60007. #60004 is fixed in CL 492743. #60007 is fixed in CL 492987 (incorporated in this CL). [Original CL 485500 description] This reapplies CL 481061, with the followup fixes in CL 482975, CL 485315, and CL 485316 incorporated. CL 481061, by doujiang24 <doujiang24@gmail.com>, speed up C to Go calls by binding the M to the C thread. See below for its description. CL 482975 is a followup fix to a C declaration in testprogcgo. CL 485315 is a followup fix for x_cgo_getstackbound on Illumos. CL 485316 is a followup cleanup for ppc64 assembly. CL 479915 passed the G to _cgo_getstackbound for direct updates to gp.stack.lo. A G can be reused on a new thread after the previous thread exited. This could trigger the C TSAN race detector because it couldn't see the synchronization in Go (lockextra) preventing the same G from being used on multiple threads at the same time. We work around this by passing the address of a stack variable to _cgo_getstackbound rather than the G. The stack is generally unique per thread, so TSAN won't see the same address from multiple threads. Even if stacks are reused across threads by pthread, C TSAN should see the synchonization in the stack allocator. A regression test is added to misc/cgo/testsanitizer. [Original CL 481061 description] This reapplies CL 392854, with the followup fixes in CL 479255, CL 479915, and CL 481057 incorporated. CL 392854, by doujiang24 <doujiang24@gmail.com>, speed up C to Go calls by binding the M to the C thread. See below for its description. CL 479255 is a followup fix for a small bug in ARM assembly code. CL 479915 is another followup fix to address C to Go calls after the C code uses some stack, but that CL is also buggy. CL 481057, by Michael Knyszek, is a followup fix for a memory leak bug of CL 479915. [Original CL 392854 description] In a C thread, it's necessary to acquire an extra M by using needm while invoking a Go function from C. But, needm and dropm are heavy costs due to the signal-related syscalls. So, we change to not dropm while returning back to C, which means binding the extra M to the C thread until it exits, to avoid needm and dropm on each C to Go call. Instead, we only dropm while the C thread exits, so the extra M won't leak. When invoking a Go function from C: Allocate a pthread variable using pthread_key_create, only once per shared object, and register a thread-exit-time destructor. And store the g0 of the current m into the thread-specified value of the pthread key, only once per C thread, so that the destructor will put the extra M back onto the extra M list while the C thread exits. When returning back to C: Skip dropm in cgocallback, when the pthread variable has been created, so that the extra M will be reused the next time invoke a Go function from C. This is purely a performance optimization. The old version, in which needm & dropm happen on each cgo call, is still correct too, and we have to keep the old version on systems with cgo but without pthreads, like Windows. This optimization is significant, and the specific value depends on the OS system and CPU, but in general, it can be considered as 10x faster, for a simple Go function call from a C thread. For the newly added BenchmarkCGoInCThread, some benchmark results: 1. it's 28x faster, from 3395 ns/op to 121 ns/op, in darwin OS & Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz 2. it's 6.5x faster, from 1495 ns/op to 230 ns/op, in Linux OS & Intel(R) Xeon(R) CPU E5-2630 0 @ 2.30GHz [CL 479915 description] Currently, when C calls into Go the first time, we grab an M using needm, which sets m.g0's stack bounds using the SP. We don't know how big the stack is, so we simply assume 32K. Previously, when the Go function returns to C, we drop the M, and the next time C calls into Go, we put a new stack bound on the g0 based on the current SP. After CL 392854, we don't drop the M, and the next time C calls into Go, we reuse the same g0, without recomputing the stack bounds. If the C code uses quite a bit of stack space before calling into Go, the SP may be well below the 32K stack bound we assumed, so the runtime thinks the g0 stack overflows. This CL makes needm get a more accurate stack bound from pthread. (In some platforms this may still be a guess as we don't know exactly where we are in the C stack), but it is probably better than simply assuming 32K. [CL 492987 description] On the first call into Go from a C thread, currently we set the g0 stack's high bound imprecisely based on the SP. With CL 485500, we keep the M and don't recompute the stack bounds when it calls into Go again. If the first call is made when the C thread uses some deep stack, but a subsequent call is made with a shallower stack, the SP may be above g0.stack.hi. This is usually okay as we don't check usually stack.hi. One place where we do check for stack.hi is in the signal handler, in adjustSignalStack. In particular, C TSAN delivers signals on the g0 stack (instead of the usual signal stack). If the SP is above g0.stack.hi, we don't see it is on the g0 stack, and throws. This CL makes it get an accurate stack upper bound with the pthread API (on the platforms where it is available). Also add some debug print for the "handler not on signal stack" throw. Fixes #51676. Fixes #59294. Fixes #59678. Fixes #60007. Change-Id: Ie51c8e81ade34ec81d69fd7bce1fe0039a470776 Reviewed-on: https://go-review.googlesource.com/c/go/+/495855 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* cmd/gofmt: try to write original data on rewrite failureIan Lance Taylor2023-05-172-26/+166
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When gofmt needs to rewrite a file, it first copies it into a backup. If the rewrite fails, it used to rename the backup to the original. However, if for some reason the file is owned by some other user, and if the rewrite fails because gofmt doesn't have permission to write to the file, then renaming the backup file will change the file owner. This CL changes gofmt so that if it fails to rewrite a file, it tries to write the original contents. If writing the original content fails, it reports the problem to the user referring to the backup file, rather than trying a rename. Also create the backup file with the correct permissions, to avoid a tiny gap when some process might get write access to the file contents that it shouldn't have. (This tiny gap only applies to files that are not formatted correctly, and have read-only permission, and are in a directory with write permission.) Fixes #60225 Change-Id: Ic16dd0c85cf416d6b2345e0650d5e64413360847 Reviewed-on: https://go-review.googlesource.com/c/go/+/495316 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Ian Lance Taylor <iant@google.com> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
* cmd/compile: build compiler with PGOCherry Mui2023-05-173-0/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reapplies CL 451292, which is reverted at CL 495475. The failure is fixed at CL 495595. Build the compiler with PGO. As go build -pgo=auto is enabled by default, we just need to store a profile in the compiler's directory. The profile is collected from building all std and cmd packages on Linux/AMD64 machine, using profile.sh. This improves the compiler speed. On Linux/AMD64, name old time/op new time/op delta Template 138ms ± 5% 136ms ± 4% -1.44% (p=0.005 n=36+39) Unicode 147ms ± 4% 140ms ± 4% -4.99% (p=0.000 n=40+39) GoTypes 780ms ± 3% 778ms ± 4% ~ (p=0.172 n=39+39) Compiler 105ms ± 5% 99ms ± 7% -5.64% (p=0.000 n=40+40) SSA 5.83s ± 6% 5.80s ± 6% ~ (p=0.556 n=40+40) Flate 89.0ms ± 5% 87.0ms ± 6% -2.18% (p=0.000 n=40+40) GoParser 172ms ± 4% 167ms ± 4% -2.72% (p=0.000 n=39+40) Reflect 333ms ± 4% 333ms ± 3% ~ (p=0.426 n=40+39) Tar 128ms ± 4% 126ms ± 4% -1.82% (p=0.000 n=39+39) XML 173ms ± 4% 170ms ± 4% -1.39% (p=0.000 n=39+40) [Geo mean] 253ms 248ms -2.13% The profile is pretty transferable. Using the same profile, we see a bigger win on Darwin/ARM64, name old time/op new time/op delta Template 71.0ms ± 2% 68.3ms ± 2% -3.90% (p=0.000 n=20+20) Unicode 71.8ms ± 2% 66.8ms ± 2% -6.90% (p=0.000 n=20+20) GoTypes 444ms ± 1% 428ms ± 1% -3.53% (p=0.000 n=19+20) Compiler 48.9ms ± 3% 45.6ms ± 3% -6.81% (p=0.000 n=20+20) SSA 3.25s ± 2% 3.09s ± 1% -5.03% (p=0.000 n=19+20) Flate 44.0ms ± 2% 42.3ms ± 2% -3.72% (p=0.000 n=19+20) GoParser 76.7ms ± 1% 73.5ms ± 1% -4.15% (p=0.000 n=18+19) Reflect 172ms ± 1% 165ms ± 1% -4.13% (p=0.000 n=20+19) Tar 63.1ms ± 1% 60.4ms ± 2% -4.24% (p=0.000 n=19+20) XML 83.2ms ± 2% 79.2ms ± 2% -4.79% (p=0.000 n=20+20) [Geo mean] 127ms 121ms -4.73% Change-Id: Ifbe35e48b3ea3b29430249b4667d2df8a2515aeb Reviewed-on: https://go-review.googlesource.com/c/go/+/495596 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* cmd/link, internal/abi: minor follow-up cleanupsDavid Chase2023-05-171-2/+2
| | | | | | | | | | | | these address comments on CLs in the large refactoring stack recently submitted. Change-Id: Ic9023c32aafe4dda953b42c9a36834d3ab3432eb Reviewed-on: https://go-review.googlesource.com/c/go/+/495835 Reviewed-by: Than McIntosh <thanm@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
* cmd/dist: add -json flagAustin Clements2023-05-173-14/+344
| | | | | | | | | | | | | | | | | | | This enables JSON output for all tests run by dist. Most the complexity here is that, in order to disambiguate JSON records from different package variants, we have to rewrite the JSON stream on the fly to include variant information. We do this by rewriting the Package field to be pkg:variant so existing CI systems will naturally pick up the disambiguated test name. Fixes #37486. Change-Id: I0094e5e27b3a02ffc108534b8258c699ed8c3b87 Reviewed-on: https://go-review.googlesource.com/c/go/+/494958 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/dist: refactor rtPreFunc to print skips in only one placeAustin Clements2023-05-171-21/+22
| | | | | | | | | | | | | | | | | Currently, all uses of rtPreFunc are to print a message and skip a test. When we move to JSON, the logic to just "print a message" is going to be more complicated, so refactor this so the function returns the skip message and we print it in just one place. We also rename the option to rtSkipFunc to better represent what we use it for. For #37486. Change-Id: Ibd537064fa646a956a1c0f85a5d8c6febd098dde Reviewed-on: https://go-review.googlesource.com/c/go/+/495856 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/dist: make it possible to filter output of background commandsAustin Clements2023-05-171-18/+24
| | | | | | | | | | | | | | | | | | | | | | | | | Many of the commands dist test executes are "background" commands run by a work queue system. The work queue allows it to run commands in parallel, but still serialize their output. Currently, the work queue system assumes that exec.Cmd.Stdout and Stderr will be nil and that it can take complete control over them. We're about to inject output filters on many of these commands, so we need a way to interpose on Stdout and Stderr. This CL rearranges responsibilities in the work queue system to make that possible. Now, the thing enqueuing the work item is responsible to constructing the Cmd to write its output to work.out. There's only one place that constructs work objects (there used to be many more), so that's relatively easy, and sets us up to add filters. For #37486. Change-Id: I55ab71ddd456a12fdbf676bb49f698fc08a5689b Reviewed-on: https://go-review.googlesource.com/c/go/+/494957 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/dist: simplify work list functionsAustin Clements2023-05-171-27/+6
| | | | | | | | | | | | | | | | | There are no uses of addCmd, so delete it. The only use of bgDirCmd is dirCmd, so inline it. Now the only function that interacts with the work queue is registerTest and dist's "background commands" are used exclusively in goTest.bgCommand and registerTest (which calls goTest.bgCommand). For #37486. Change-Id: Iebbb24cf9dbee45f3975fe9504d858493e1cd947 Reviewed-on: https://go-review.googlesource.com/c/go/+/494956 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd: update vendored golang.org/x/modDmitri Shuralyov2023-05-175-5/+63
| | | | | | | | | | | | | | | | | | | | Pull in CL 492990. This teaches 'go mod tidy' and other go subcommands that write go.mod files to use semantic sort for exclude blocks, gated on said files declaring Go version 1.21 or higher. go get golang.org/x/mod@e7bea8f1d64f # includes CL 492990 go mod tidy go mod vendor Fixes #60028. Change-Id: Ia9342dcc23cd68de068a70657b59c25f69afa381 Reviewed-on: https://go-review.googlesource.com/c/go/+/494578 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
* cmd/compile: don't inline from norace packages in race modeCherry Mui2023-05-172-1/+26
| | | | | | | | | | | | | | | | | | | In race mode (or other instrumentation mode), if the caller is in a regular package and the callee is in a norace (or noinstrument) package, don't inline. Otherwise, when the caller is instumented it will also affect the inlined callee. An example is sync.(*Mutex).Unlock, which is typically not inlined but with PGO it can be inlined into a regular function, which is then get instrumented. But the rest of the sync package, in particular, the Lock function is not instrumented, causing the race detector to signal false race. Change-Id: Ia78bb602c6da63a34ec2909b9a82646bf20873f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/495595 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/go: prune more dependencies in 'go get'Bryan C. Mills2023-05-1713-537/+1100
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prior to this change, 'go get' pulled in every version of each module whose path is explicitly listed in the go.mod file. When graph pruning is enabled (that is, when the main module is at 'go 1.17' or higher), that pulled in transitive dependencies of older-than-selected versions of dependencies, which are normally pruned out by other 'go' commands (including 'go mod tidy' and 'go mod graph'). To make matters worse, different parts of `go get` were making different assumptions about which kinds of conflicts would be reported: the modget package assumed that any conflict is necessarily due to some explicit constraint, but 'go get' was imposing an additional constraint that modules could not be incidentally upgraded in the course of a downgrade. When that additional constraint failed, the modload package reported the failure as though it were a real (caller-supplied) constraint, confusing the caller (which couldn't identify any specific package or argument that caused the failure). This change fixes both of those problems by replacing the modload.EditRequirements algorithm with a different one. The new algorithm is, roughly, as follows. 1. Propose a list of “root requirements” to be written to the updated go.mod file. 2. Load the module graph from those requirements mostly as usual, but if any root is upgraded due to transitive dependencies, retain the original roots and the paths leading from those roots to the upgrades. (This forms an “extended graph”, in which we can trace a path from to each version that appears in the graph starting at one or more of the original roots.) 3. Identify which roots caused any module path to be upgraded above its passed-in version constraint. For each such root, either report an unresolvable conflict (if the root itself is constrained to a specific version) or identify an updated version to propose: either a downgrade to the next-highest version, or an upgrade to the actually-selected version of the root (if that version is allowed). To avoid looping forever or devolving into an NP-complete search, we never propose a version that was already rejected previously, regardless of what other roots were present alongside it at the time. 4. If the version of any root was changed, repeat from (1). This algorithm is guaranteed to terminate, because there are finitely many root versions and we permanently reject at least one each time we downgrade its path to a lower version. In addition, this change implements support for the '-v' flag to log more information about version changes at each iteration. Fixes #56494. Fixes #55955. Change-Id: Iebc17dd7586594d5732e228043c3c4c6da230f44 Reviewed-on: https://go-review.googlesource.com/c/go/+/471595 TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
* cmd/go: add a test that reproduces the root cause of issue #56494Bryan C. Mills2023-05-172-1/+137
| | | | | | | | | | | For #56494. Change-Id: I9bbded6d014ac73d81b973f2d7b4783e64380031 Reviewed-on: https://go-review.googlesource.com/c/go/+/447797 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Michael Matloob <matloob@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com>
* go/types, types2: permit partially instantiated functions as function argumentsRobert Griesemer2023-05-163-51/+148
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This CL changes Checker.genericExprList such that it collects partially instantiated generic functions together with their (partial) type argument (and corresponding) expression lists, instead of trying to infer the missing type arguments in place or to report an error. Special care is being taken to explictly record expression types where needed (because we can't use one of the usual expr evaluators which takes care of that), or to track the correct instance expression for later recording with Checker.arguments. The resulting generic expression list is passed to Checker.arguments which is changed to accept explicit partial type argument (and corresponding) expression lists. The provided type arguments are fed into type inference, matching up with their respective type parameters (which were collected already, before this CL). If type inference is successful, the instantiated functions are recorded as needed. For now, the type argument expression lists are collected and passed along but not yet used. We may use them eventually for better error reporting. Fixes #59958. For #59338. Change-Id: I26db47ef3546e64553da49d62b23cd3ef9e2b549 Reviewed-on: https://go-review.googlesource.com/c/go/+/494116 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Griesemer <gri@google.com>
* go/types, types2: remove superfluous argument test in Checker.argumentsRobert Griesemer2023-05-161-11/+0
| | | | | | | | | | | | | | | | | | | | | | | | There's only two places which call Checker.arguments: Checker.callExpr and Checker.builtin. Both ensure that the passed argument list doesn't contain type expressions, so we don't need that extra check at the start of Checker.arguments. The remaining check causes Checker.arguments to exit early if any of the passed arguments is invalid. This reduces the number of reported errors in rare cases but is executed all the time. If the extra errors are a problem, it would be better to not call Checker.arguments in the first place, or only do the extra check before Checker.arguments reports an error. Removing this code for now. Removes a long-standing TODO. Change-Id: Ief654b680eb6b6a768bb1b4c621d3c8169953f17 Reviewed-on: https://go-review.googlesource.com/c/go/+/495395 Run-TryBot: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
* cmd/compile: call phiElimValue from removePhiArgJakub Ciolek2023-05-164-8/+2
| | | | | | | | | | | | | | | | | With the exception of the shortcircuit pass, removePhiArg is always unconditionally followed by phiElimValue. Move the phiElimValue inside removePhiArg. Resolves a TODO. See CL 357964 for more info. Change-Id: I8460b35864f4cd7301ba86fc3dce08ec8041da7f Reviewed-on: https://go-review.googlesource.com/c/go/+/465435 Reviewed-by: David Chase <drchase@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
* cmd/compile/internal/noder: suppress unionType consistency checkMatthew Dempsky2023-05-161-12/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | In the types1 universe, we only need to represent value types. For interfaces, this means we only need to worry about pure interfaces. A pure interface can embed a union type, but the overall union must be equivalent to "any". In go.dev/cl/458619, we changed the types1 reader to return "any", but to incorporate a consistency check to make sure this is valid. Unfortunately, a pure interface can actually still reference impure interfaces, and in general this is hard to check precisely without reimplementing a lot of types2 data structures and logic into types1. We haven't had any other reports of this check failing since 1.20, so it seems simplest to just suppress for now. Fixes #60117. Change-Id: I5053faafe2d1068c6d438b2193347546bf5330cd Reviewed-on: https://go-review.googlesource.com/c/go/+/495455 Run-TryBot: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
* Revert "cmd/compile: build compiler with PGO"Cherry Mui2023-05-163-25/+0
| | | | | | | | | | | | | This reverts CL 451292. Reason for revert: causes the racecompile builder failure. https://build.golang.org/log/32d2fc21bd6e3bd415495d04befe806c0f10ea8b Change-Id: I5863437d4b814712b1280a1c21ba86009c332645 Reviewed-on: https://go-review.googlesource.com/c/go/+/495475 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
* cmd/compile: build compiler with PGOCherry Mui2023-05-163-0/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Build the compiler with PGO. As go build -pgo=auto is enabled by default, we just need to store a profile in the compiler's directory. The profile is collected from building all std and cmd packages on Linux/AMD64 machine, using profile.sh. This improves the compiler speed. On Linux/AMD64, name old time/op new time/op delta Template 138ms ± 5% 136ms ± 4% -1.44% (p=0.005 n=36+39) Unicode 147ms ± 4% 140ms ± 4% -4.99% (p=0.000 n=40+39) GoTypes 780ms ± 3% 778ms ± 4% ~ (p=0.172 n=39+39) Compiler 105ms ± 5% 99ms ± 7% -5.64% (p=0.000 n=40+40) SSA 5.83s ± 6% 5.80s ± 6% ~ (p=0.556 n=40+40) Flate 89.0ms ± 5% 87.0ms ± 6% -2.18% (p=0.000 n=40+40) GoParser 172ms ± 4% 167ms ± 4% -2.72% (p=0.000 n=39+40) Reflect 333ms ± 4% 333ms ± 3% ~ (p=0.426 n=40+39) Tar 128ms ± 4% 126ms ± 4% -1.82% (p=0.000 n=39+39) XML 173ms ± 4% 170ms ± 4% -1.39% (p=0.000 n=39+40) [Geo mean] 253ms 248ms -2.13% The profile is pretty transferable. Using the same profile, we see a bigger win on Darwin/ARM64, name old time/op new time/op delta Template 71.0ms ± 2% 68.3ms ± 2% -3.90% (p=0.000 n=20+20) Unicode 71.8ms ± 2% 66.8ms ± 2% -6.90% (p=0.000 n=20+20) GoTypes 444ms ± 1% 428ms ± 1% -3.53% (p=0.000 n=19+20) Compiler 48.9ms ± 3% 45.6ms ± 3% -6.81% (p=0.000 n=20+20) SSA 3.25s ± 2% 3.09s ± 1% -5.03% (p=0.000 n=19+20) Flate 44.0ms ± 2% 42.3ms ± 2% -3.72% (p=0.000 n=19+20) GoParser 76.7ms ± 1% 73.5ms ± 1% -4.15% (p=0.000 n=18+19) Reflect 172ms ± 1% 165ms ± 1% -4.13% (p=0.000 n=20+19) Tar 63.1ms ± 1% 60.4ms ± 2% -4.24% (p=0.000 n=19+20) XML 83.2ms ± 2% 79.2ms ± 2% -4.79% (p=0.000 n=20+20) [Geo mean] 127ms 121ms -4.73% Change-Id: I44735b3f7fd6903efbbe6b19c05dee874bea4c89 Reviewed-on: https://go-review.googlesource.com/c/go/+/451292 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/compile: add more information to the bisect-verbose reportDavid Chase2023-05-162-5/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | running on cmd/compile/internal/testdata/inlines now shows: ``` --- change set #1 (enabling changes causes failure) b/b.go:16:6: loop variable i now per-iteration (loop inlined into b/b.go:10) b/b.go:16:6: loop variable i now per-iteration ./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into a/a.go:18) ./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:37) ./b/b.go:16:6: loop variable b.i now per-iteration (loop inlined into ./main.go:38) --- ``` and ``` --- change set #2 (enabling changes causes failure) ./main.go:27:6: loop variable i now per-iteration ./main.go:27:6: loop variable i now per-iteration (loop inlined into ./main.go:35) --- ``` Still unsure about the utility of mentioning the inlined occurrence, but better than mysteriously repeating the line over and over again. Change-Id: I357f5d419ab4928fa316f4612eec3b75e7f8ac34 Reviewed-on: https://go-review.googlesource.com/c/go/+/494296 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Run-TryBot: David Chase <drchase@google.com>
* cmd/link/internal/ppc64: link ELFv2 objects built with -mcpu=power10Paul E. Murphy2023-05-161-51/+253
| | | | | | | | | | | | | | | | | | | | | | | | | | Specifically, objects built via cgo using CGO_CFLAGS="-O2 -g -mcpu=power10". These use new relocations defined by ELFv2 1.5, and the R_PPC64_REL24_NOTOC relocation. These objects contain functions which may not use a TOC pointer requiring the insertion of trampolines to use correctly. The relocation targets of these ELFv2 objects may also contain non-zero values. Clear the relocated bits before setting them. Extra care is taken if GOPPC64 < power10. The R_PPC64_REL24_NOTOC reloc existed prior to ELFv2 1.5. The presence of this relocation itself does not imply a power10 target. Generate power8 compatible stubs if GOPPC64 < power10. Updates #44549 Change-Id: I06ff8c4e47ed9af835a7dcfbafcfa4c538f75544 Reviewed-on: https://go-review.googlesource.com/c/go/+/492617 Run-TryBot: Paul Murphy <murp@ibm.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/compile: make memcombine pass a bit more robust to reassociation of exprsKeith Randall2023-05-161-24/+17
| | | | | | | | | | | | | | | Be more liberal about expanding the OR tree. Handle any tree shape instead of a fully left or right associative tree. Also remove tail feature, it isn't ever needed. Change-Id: If16bebef94b952a604d6069e9be3d9129994cb6f Reviewed-on: https://go-review.googlesource.com/c/go/+/494056 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Ryan Berger <ryanbberger@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
* cmd/compile/internal/walk: delete statement that don't needcuiweixie2023-05-161-1/+0
| | | | | | | | | Change-Id: I7253aed4808a06379caebf0949aec0f305245d23 Reviewed-on: https://go-review.googlesource.com/c/go/+/494835 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: xie cui <523516579@qq.com>
* cmd/dist: introduce test variantsAustin Clements2023-05-161-33/+79
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This introduces the concept of test variants in dist, which are different configurations of the same package. The variant of a test is a short string summarizing the configuration. The "variant name" of a test is either the package name if the variant is empty, or package:variant if not. Currently this isn't used for anything, but soon we'll use this as the Package field of the test JSON output so that we can disambiguate output from differently configured runs of the same test package, and naturally flow this through to any test result viewer. The long-term plan is to use variant names as dist's own test names and eliminate the ad hoc names it has right now. Unfortunately, the build coordinator is aware of many of the ad hoc dist test names, so some more work is needed to get to that point. This CL keeps almost all test names the same, with the exception of tests registered by registerCgoTests, where we regularize test names a bit using variants to avoid some unnecessary complexity (I believe nothing depends on the names of these tests). For #37486. Change-Id: I119fec2872e40b12c1973cf2cddc7f413d62a48c Reviewed-on: https://go-review.googlesource.com/c/go/+/495016 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/dist: put cgo tests under a "Testing cgo" headingAustin Clements2023-05-161-26/+24
| | | | | | | | | | | | | | | | | | | Currently the cgo tests mostly use their package name as a heading, which means we get a large number of test sections that each have a single test package in them. Unify them all under "Testing cgo" to reduce output noise. This leaves just the cmd/api test without a heading, so we give it a heading and require that all tests have a heading. Change-Id: I24cd9a96eb35bbc3ff9335ca8a382ec2426306c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/494497 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
* cmd/dist: refactor adding to the test list into a methodAustin Clements2023-05-161-103/+92
| | | | | | | | | | | | | | | | | | Currently, there are four places that add distTests to the tester.tests list. That means we're already missing a few name uniqueness checks, and we're about to start enforcing some more requirements on tests that would be nice to have in one place. Hence, to prepare for this, this CL refactors the process of adding to the tester.tests list into a method. That also means we can trivially use a map to check name uniqueness rather than an n^2 slice search. For #37486. Change-Id: Ib7b64c7bbf65e5e870c4f4bfaca8c7f70983605c Reviewed-on: https://go-review.googlesource.com/c/go/+/495015 Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/compile: enhance tighten pass for memory valueseric fang2023-05-162-7/+105
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | [This is a roll-forward of CL 458755, which was reverted due to make.bash being broken on GOAMD64=v3. But it turned out that the problem was caused by wrong bswap/load rewrite rules, and it was fixed in CL 492616.] This CL enhances the tighten pass. Previously if a value has memory arg, then the tighten pass won't move it, actually if the memory state is consistent among definition and use block, we can move the value. This CL optimizes this case. This is useful for the following situation: b1: x = load(...mem) if(...) goto b2 else b3 b2: use(x) b3: some_op_not_use_x For the micro-benchmark mentioned in #56620, the performance improvement is about 15%. There's no noticeable performance change in the go1 benchmark. Fixes #56620 Change-Id: I36ea68bed384986cd3ae81cb9e6efe84bb213adc Reviewed-on: https://go-review.googlesource.com/c/go/+/492895 Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Eric Fang <eric.fang@arm.com>
* cmd/compile: update rules to generate more prefixed instructionsLynn Boger2023-05-153-141/+159
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This modifies some existing rules to allow more prefixed instructions to be generated when using GOPPC64=power10. Some rules also check if PCRel is available, which is currently supported for linux/ppc64le and linux/ppc64 (internal linking only). Prior to p10, DS-offset loads and stores had a 16 bit size limit for the offset field. If the offset of the data for load or store was beyond this range then an indexed load or store would be selected by the rules. In p10 the assembler can generate prefixed instructions in this case, but does not if an indexed instruction was selected during the lowering pass. This allows many more cases to use prefixed loads or stores, reducing function sizes and improving performance in some cases where the code change happens in key loops. For example in strconv BenchmarkAppendQuoteRune before: 12c5e4: 15 00 10 06 pla r10,1425660 12c5e8: fc c0 40 39 12c5ec: 00 00 6a e8 ld r3,0(r10) 12c5f0: 10 00 aa e8 ld r5,16(r10) After this change: 12a828: 15 00 10 04 pld r3,1433272 12a82c: b8 de 60 e4 12a830: 15 00 10 04 pld r5,1433280 12a834: c0 de a0 e4 Performs better in the second case. A testcase was added to verify that the rules correctly select a load or store based on the offset and whether power10 or earlier. Change-Id: I4335fed0bd9b8aba8a4f84d69b89f819cc464846 Reviewed-on: https://go-review.googlesource.com/c/go/+/477398 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Archana Ravindar <aravind5@in.ibm.com> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Paul Murphy <murp@ibm.com>
* cmd/go/internal/modload: replace import error message from goroot to stdjchen0382023-05-158-14/+14
| | | | | | | | | | | | | | | When importing a package that does not exist, it would show goroot error message and path. We would like to replace goroot with std instead. Fixes #56965. Change-Id: I86f8a7fab6555b68f792a3a4686de20d51eced8b Reviewed-on: https://go-review.googlesource.com/c/go/+/453895 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/cgo: enable test with non-sensible build tagAustin Clements2023-05-151-1/+1
| | | | | | | | | | | | | | | The build tag on this file is currently unsatisfiable. It was clearly supposed to be "linux || freebsd || openbsd", but the test doesn't actually compile on FreeBSD or OpenBSD because they don't define SYS_gettid. Change the build tag to just "linux". Change-Id: Ifaffac5438e1b94a8588b5a00435461aa171a6fc Reviewed-on: https://go-review.googlesource.com/c/go/+/493603 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/go/internal/modindex: update index_format.txtBryan C. Mills2023-05-132-8/+14
| | | | | | | | | | | | | | | | This incorporates the changes from CL 453603 and CL 416178. Please review carefully: I did my best to read through the CLs, but I'm not entirely confident I haven't made a mistake. Fixes #59770. Change-Id: Ib8937e55dcd11e3f75c16b28519d3d91df1d4da3 Reviewed-on: https://go-review.googlesource.com/c/go/+/492596 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
* cmd/link: fix checks for supported linker flags with relative paths.James Bartlett2023-05-121-2/+3
| | | | | | | | | | | | | | | | The existing way of checking for supported linker flags causes false negatives when there are relative paths passed to go tool link. This fixes the issue by calling the external linker in the current working directory, instead of in a temporary directory. Fixes #59952 Change-Id: I173bb8b44902f30dacefde1c202586f87667ab70 Reviewed-on: https://go-review.googlesource.com/c/go/+/491796 Run-TryBot: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
* cmd/dist: use registerStdTestSpecially for normal Go tests onlyDmitri Shuralyov2023-05-121-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | It was my oversight in CL 463276 to skip registerStdTestSpecially packages in the race bench test registration loop. Package testdir has no benchmarks and doesn't need to be skipped. (And if it had benchmarks, it's very unlikely they'd need any special handling.) By now there are more cmd/cgo/internal/... packages that are registered specially, and there isn't a need for their few benchmarks not to be used for the purpose of race bench tests. If the 3 benchmarks in cmd/cgo/internal/test were to require something special, then we can add it to a new registerRaceBenchTestSpecially map with a comment, and do register them specially in registerTests instead of forgetting to. This restores the automatic 'go_test_bench:cmd/cgo/internal/test' registration and reduces prevalence of registerStdTestSpecially a bit. For #37486. For #56844. Change-Id: I1791fe5bf94cb4b4e0859c5fff4e7a3d5a23723e Reviewed-on: https://go-review.googlesource.com/c/go/+/494656 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
* reflect: do not escape Value.TypeCherry Mui2023-05-121-5/+13
| | | | | | | | | | | | | | | | | | | Types are either static (for compiler-created types) or heap allocated and always reachable (for reflection-created types, held in the central map). So there is no need to escape types. With CL 408826 reflect.Value does not always escape. Some functions that escapes Value.typ would make the Value escape without this CL. Had to add a special case for the inliner to keep (*Value).Type still inlineable. Change-Id: I7c14d35fd26328347b509a06eb5bd1534d40775f Reviewed-on: https://go-review.googlesource.com/c/go/+/413474 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/compile: remove post-inlining PGO graph dumpMichael Pratt2023-05-122-100/+0
| | | | | | | | | | | | | | | | | | | The RedirectEdges logic is fragile and not quite complete (doesn't update in-edges), which adds overhead to maintaining this package. In my opinion, the post-inlining graph doesn't provide as much value as the pre-inlining graph. Even the latter I am not convinced should be in the compiler rather than an external tool, but it is comparatively easier to maintain. Drop it for now. Perhaps we'll want it back in the future for tracking follow-up optimizations, but for now keep things simple. Change-Id: I3133a2eb97893a14a6770547f96a3f1796798d17 Reviewed-on: https://go-review.googlesource.com/c/go/+/494655 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Michael Pratt <mpratt@google.com>
* cmd/compile/internal/pgo: remove node weights from IRNodeMichael Pratt2023-05-121-66/+32
| | | | | | | | | | | | | | | | | | | | | | | Actual PGO operation doesn't use these weights at all. They are theoretically used when printing a dot graph for debugging, but that doesn't actually work because these weights are always zero. These fields are initialized by looking for a NodeMap entry with key {CallerName: sym, CalleeName: "", CallSiteOffset: 0}. These entries will never exist, as we never put entries in NodeMap without CalleeName. Since they aren't really used and don't work, just remove them entirely, which offers nice simplification. This leaves IRNode with just a single field. I keep the type around as a future CL will make the *ir.Func optional, allowing nodes with a name but no IR. Change-Id: I1646654cad1d0779ce071042768ffad2a7e6ff49 Reviewed-on: https://go-review.googlesource.com/c/go/+/494616 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Michael Pratt <mpratt@google.com>
* cmd/cgo/internal/testcarchive: build on all platformsAustin Clements2023-05-121-0/+5
| | | | | | | | | | | | | | | | | | | | | | This test package uses syscall.SIGSEGV and syscall.SIGPIPE, which are defined on most, but not all platforms. Normally this test runs as part of dist test, which only registers this test on platforms that support c-archive build mode, which includes all platforms that define these signals. But this doesn't help if you're just trying to type check everything in cmd. Add build constraints so that this package type checks on all platforms. Fixes #60164. Updates #37486. Change-Id: Id3f9ad4cc9f80146de16aedcf85d108a77215ae6 Reviewed-on: https://go-review.googlesource.com/c/go/+/494659 Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Austin Clements <austin@google.com> Auto-Submit: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/cgo/internal/testsanitizers: build on all platformsAustin Clements2023-05-128-5/+24
| | | | | | | | | | | | | | | | | | | | | | This test package uses the Pdeathsig field of syscall.SysProcAttr, which is only available on a few platforms. Currently, dist test checks for compatible platforms and only registers it as part of all.bash on platforms where it can build. But this doesn't help if you're just trying to type check everything in cmd. Make this package pass type checking by moving the condition from dist into build tags on the test package itself. For #60164. Updates #37486. Change-Id: I58b12d547c323cec895320baa5fca1b82e99d1b5 Reviewed-on: https://go-review.googlesource.com/c/go/+/494658 Auto-Submit: Austin Clements <austin@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Austin Clements <austin@google.com>
* cmd/internal/bootstrap_test: update TestExperimentToolID for Go 1.21Dmitri Shuralyov2023-05-122-29/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | This test is configured to run only when explicitly requested due to being costly. Apply two updates so it can run on the toolchain today: - overlay GOROOT/lib for zoneinfo.zip (similarly to CL 462279) - stop expecting framepointer to be listed in the GOEXPERIMENT section of the compiler version (see CL 49252 and CL 249857) I checked if by now there's another test that would report a problem if the fix made in CL 186200 had regressed. Running all.bash locally with GO_TEST_SHORT=0 GO_BUILDER_NAME=darwin-arm64-longtest passed ok, while this manual test did catch the problem. Also simplify the test implementation while here so it's less different from TestRepeatBootstrap. For #33091. Change-Id: I14eea18c19c2e8996bcba31c80e03dcf679f56ab Reviewed-on: https://go-review.googlesource.com/c/go/+/493475 Reviewed-by: Bryan Mills <bcmills@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* cmd/go: add a regression test for Git configurations with ↵Bryan C. Mills2023-05-121-0/+19
| | | | | | | | | | | | safe.bareRepository=explicit Change-Id: I394265a4bf849ec89ac44c67aeaaaca801e46caa Reviewed-on: https://go-review.googlesource.com/c/go/+/493476 Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* cmd/go/internal/modload: reject the -modfile flag in workspace modeZeke Lu2023-05-127-14/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, in workspace mode, the -modfile flag affects all the modules listed in the go.work file. This is not desirable most of the time. And when it results in an error, the error message does not help. For example, when there are more than one modules listed in the go.work file, running "go list -m -modfile=path/to/go.mod" gives this error: go: module example.com/foo appears multiple times in workspace This change reject -modfile flag explicitly with this error message: go: -modfile cannot be used in workspace mode While at here, correct some typos in the modload package. Fixes #59996. Change-Id: Iff4cd9f3974ea359889dd713a747b6932cf42dfd GitHub-Last-Rev: 7dbc9c3f2f9bfe8acab088eb3266a08d8ec1ba16 GitHub-Pull-Request: golang/go#60033 Reviewed-on: https://go-review.googlesource.com/c/go/+/493315 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com>
* internal/testdir: move to cmd/internal/testdirDmitri Shuralyov2023-05-122-2/+1941
| | | | | | | | | | | | | | | | | | The effect and motivation is for the test to be selected when doing 'go test cmd' and not when doing 'go test std' since it's primarily about testing the Go compiler and linker. Other than that, it's run by all.bash and 'go test std cmd' as before. For #56844. Fixes #60059. Change-Id: I2d499af013f9d9b8761fdf4573f8d27d80c1fccf Reviewed-on: https://go-review.googlesource.com/c/go/+/493876 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
* go/types: minor refactoring of missingMethod following CL 494615Rob Findley2023-05-121-4/+7
| | | | | | | | | | Make the refactoring suggested by gri@ in that CL. Change-Id: I6c363f3ba5aaa3c616d3982d998b989de7046a86 Reviewed-on: https://go-review.googlesource.com/c/go/+/494617 Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* go/types, types2: call recordInstance in instantiateSignatureRobert Griesemer2023-05-121-19/+12
| | | | | | | | | | | | | This matches the pattern we use for type instantiations and factors out the recordInstance and associated assert calls. Change-Id: Ib7731c0e619aca42f418cb2d9a153785aaf014cb Reviewed-on: https://go-review.googlesource.com/c/go/+/494457 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com> Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Run-TryBot: Robert Griesemer <gri@google.com>
* go/types, types2: be sure to type-check wrong methods in missingMethodRob Findley2023-05-121-0/+4
| | | | | | | | | | | | | | | | In the case of a wrong method, we were not ensuring that it was type-checked before passing it to funcString. Formatting the missing method error message requires a fully set-up signature. Fixes #59848 Change-Id: I1467e036afbbbdd00899bfd627a945500dc709c2 Reviewed-on: https://go-review.googlesource.com/c/go/+/494615 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
* cmd/go: diff .so files quietly in TestScript/build_plugin_reproducibleDmitri Shuralyov2023-05-121-1/+1
| | | | | | | | | | | | | | | This avoids printing verbose binary data and making bell sounds when the test fails. The binary data can be inspected via other means if needed. For #58557. Change-Id: Ia1c4f2c6b9ff2cf6f97611cf335b978fc7bb201f Reviewed-on: https://go-review.googlesource.com/c/go/+/494577 Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com>
* cmd/dist: drop goTest.dirAustin Clements2023-05-121-29/+23
| | | | | | | | | | | | At this point all tests are regular packages in std or cmd, so we no longer need goTest.dir. Change-Id: I46a0c7b4464b0738e9959e41bf840ba1b73e3590 Reviewed-on: https://go-review.googlesource.com/c/go/+/494194 Run-TryBot: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
* test/bench: deleteAustin Clements2023-05-122-6/+0
| | | | | | | | | | | | | | | | | | | | Russ added test/bench/go1 in CL 5484071 to have a stable suite of programs to use as benchmarks. For the compiler and runtime we had back then, those were reasonable benchmarks, but the compiler and runtime are now far more sophisticated and these benchmarks no longer have good coverage. We also now have better benchmark suites maintained outside the repo (e.g., golang.org/x/benchmarks). Keeping test/bench/go1 at this point is actively misleading. Indirectly related to #37486, as this also removes the last package dist test runs outside of src/. Change-Id: I2867ef303fe48a02acce58ace4ee682add8acdbf Reviewed-on: https://go-review.googlesource.com/c/go/+/494193 Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
* misc/reboot: move to cmd/internal/bootstrap_testAustin Clements2023-05-124-7/+285
| | | | | | | | | | | | | This is the last test run from misc by dist. For #37486. Change-Id: I1a70ded29ba0de548c9a16611ba987a258121e80 Reviewed-on: https://go-review.googlesource.com/c/go/+/493606 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>