Skip to content

Commit

Permalink
cmd/go: fix go command fails to perform concurrent compilation
Browse files Browse the repository at this point in the history
CL 344909 fixed the bug in the order of passing gcflags from cmd/go to
cmd/compile. In that process, we merged the flags passed by cmd/go and
the flags specified by "-gcflags" to one variable. That causes the
gcBackendConcurrency function fails to detect concurrency level, since
when it expects only the latter flags.

To fix this, just don't merge those two variables, so we can now
correctly detect the concurrency level and passing the right -c to
the compiler.

Fixes #48490

Change-Id: I1293a7d6b946b7fccdd5cd34a38452bf6306e115
Reviewed-on: https://go-review.googlesource.com/c/go/+/351049
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
  • Loading branch information
cuonglm committed Sep 22, 2021
1 parent 3664950 commit 085c609
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
24 changes: 12 additions & 12 deletions src/cmd/go/internal/work/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
}

pkgpath := pkgPath(a)
gcflags := []string{"-p", pkgpath}
defaultGcFlags := []string{"-p", pkgpath}
if p.Module != nil {
v := p.Module.GoVersion
if v == "" {
Expand All @@ -94,19 +94,19 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
v = "1.16"
}
if allowedVersion(v) {
gcflags = append(gcflags, "-lang=go"+v)
defaultGcFlags = append(defaultGcFlags, "-lang=go"+v)
}
}
if p.Standard {
gcflags = append(gcflags, "-std")
defaultGcFlags = append(defaultGcFlags, "-std")
}
_, compilingRuntime := runtimePackages[p.ImportPath]
compilingRuntime = compilingRuntime && p.Standard
if compilingRuntime {
// runtime compiles with a special gc flag to check for
// memory allocations that are invalid in the runtime package,
// and to implement some special compiler pragmas.
gcflags = append(gcflags, "-+")
defaultGcFlags = append(defaultGcFlags, "-+")
}

// If we're giving the compiler the entire package (no C etc files), tell it that,
Expand All @@ -125,25 +125,25 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
}
}
if extFiles == 0 {
gcflags = append(gcflags, "-complete")
defaultGcFlags = append(defaultGcFlags, "-complete")
}
if cfg.BuildContext.InstallSuffix != "" {
gcflags = append(gcflags, "-installsuffix", cfg.BuildContext.InstallSuffix)
defaultGcFlags = append(defaultGcFlags, "-installsuffix", cfg.BuildContext.InstallSuffix)
}
if a.buildID != "" {
gcflags = append(gcflags, "-buildid", a.buildID)
defaultGcFlags = append(defaultGcFlags, "-buildid", a.buildID)
}
if p.Internal.OmitDebug || cfg.Goos == "plan9" || cfg.Goarch == "wasm" {
gcflags = append(gcflags, "-dwarf=false")
defaultGcFlags = append(defaultGcFlags, "-dwarf=false")
}
if strings.HasPrefix(runtimeVersion, "go1") && !strings.Contains(os.Args[0], "go_bootstrap") {
gcflags = append(gcflags, "-goversion", runtimeVersion)
defaultGcFlags = append(defaultGcFlags, "-goversion", runtimeVersion)
}
if symabis != "" {
gcflags = append(gcflags, "-symabis", symabis)
defaultGcFlags = append(defaultGcFlags, "-symabis", symabis)
}

gcflags = append(gcflags, str.StringList(forcedGcflags, p.Internal.Gcflags)...)
gcflags := str.StringList(forcedGcflags, p.Internal.Gcflags)
if compilingRuntime {
// Remove -N, if present.
// It is not possible to build the runtime with no optimizations,
Expand All @@ -157,7 +157,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
}
}

args := []interface{}{cfg.BuildToolexec, base.Tool("compile"), "-o", ofile, "-trimpath", a.trimpath(), gcflags}
args := []interface{}{cfg.BuildToolexec, base.Tool("compile"), "-o", ofile, "-trimpath", a.trimpath(), defaultGcFlags, gcflags}
if p.Internal.LocalPrefix != "" {
// Workaround #43883.
args = append(args, "-D", p.Internal.LocalPrefix)
Expand Down
14 changes: 14 additions & 0 deletions src/cmd/go/testdata/script/build_concurrent_backend.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Tests golang.org/issue/48490
# cmd/go should enable concurrent compilation by default

# Skip test on darwin/arm64, see #48496.
# TODO(cuonglm): remove this once #48496 is fixed.
[darwin] [arm64] skip

# Reset all experiments, since one of them can disable
# concurrent compilation, e.g: fieldtrack.
env GOEXPERIMENT=none

env GOMAXPROCS=4
go build -n -x -a fmt
stderr ' -c=4 '

0 comments on commit 085c609

Please sign in to comment.