Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gccgo: cmd/internal/obj/x86 takes a long time to compile #46600

Closed
mdempsky opened this issue Jun 6, 2021 · 10 comments
Closed

gccgo: cmd/internal/obj/x86 takes a long time to compile #46600

mdempsky opened this issue Jun 6, 2021 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Jun 6, 2021

Fedora on amd64 uses gccgo as the default Go implementation installed to /usr, and using this to bootstrap the gc implementation takes a very long time due to compiling cmd/internal/obj/x86:

matthew@bento:~/wd/go/pkg/bootstrap/src/bootstrap/cmd/internal/obj/x86$ time /usr/bin/gccgo -c -O2 -g -m64 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -fgo-pkgpath=bootstrap/cmd/internal/obj/x86 -o $WORK/b072/_go_.o -I $WORK/b072/_importcfgroot_ ./a.out.go ./aenum.go ./anames.go ./asm6.go ./avx_optabs.go ./evex.go ./list6.go ./obj6.go ./ytab.go

real	7m0.867s
user	6m59.107s
sys	0m0.377s

matthew@bento:~/wd/go/pkg/bootstrap/src/bootstrap/cmd/internal/obj/x86$ /usr/bin/gccgo -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gccgo
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl=/builddir/build/BUILD/gcc-11.1.1-20210428/obj-x86_64-redhat-linux/isl-install --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --enable-cet --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.1.1 20210428 (Red Hat 11.1.1-1) (GCC) 

/cc @ianlancetaylor

@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 6, 2021
@mdempsky mdempsky added this to the Gccgo milestone Jun 6, 2021
@mdempsky
Copy link
Contributor Author

mdempsky commented Jun 7, 2021

Compiling cmd/compile/internal/ssa also takes about 90 seconds. More understandable since ssa is a large package with a lot of code, but mentioning in case there's anything to do about it too.

@thanm
Copy link
Contributor

thanm commented Jun 8, 2021

I can reproduce this on tip, more or less.

On my machine the "bootstrap/cmd/internal/obj/x86" build takes about 257 seconds at "-O2" and 450 seconds at "-O1".

The offending pass appears to be variable_tracking; looks like the time is being spent in alias analysis / memory disambiguation.

Attaching a couple of profiles.

graph
tree.txt
top10.txt

@thanm
Copy link
Contributor

thanm commented Jun 13, 2021

As an experiment, I carved off the file avx_optabs.go from the package and put it into its own stand-alone module. The new package takes about 3 minutes to compile at -O1 and 1.5 minutes at -O2.

I also did a bit of experimenting with GCC to see if this is a regression or whether the problem has been around for a while. Turns out that this behavior has been there for a good long while. I tested (using release branches) gcc-8, gcc-9, gcc-10, and gcc-11, and I'm seeing long compile times for all of them (gcc-11 is maybe 5 or 10 percent faster, but that's it). So I don't think this is a situation where we can bisect it to a specific GCC commit.

@thanm
Copy link
Contributor

thanm commented Jun 14, 2021

I filed an upstream GCC bug for this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101064

@mdempsky
Copy link
Contributor Author

@thanm Thanks for investigating and filing the GCC issue. Do you see any non-invasive tweaks we could make to the cmd/internal/obj/x86 sources to mitigate this issue and get better compile times under gccgo? E.g., could we break up the variable initializers into explicit initialization functions or something?

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/331513 mentions this issue: compiler: in composite literals use temps only for interfaces

@thanm
Copy link
Contributor

thanm commented Jun 29, 2021

Do you see any non-invasive tweaks we could make to the cmd/internal/obj/x86 sources to mitigate this issue and get better compile times under gccgo?

I was going to suggest breaking out avx_optabs into a separate package (I prototyped that to see if if would work), which would help reduce the critical path. Looks like Ian has sent a gofrontend change however.

gopherbot pushed a commit to golang/gofrontend that referenced this issue Jun 29, 2021
For a composite literal we only need to introduce a temporary variable
if we may be converting to an interface type, so only do it then.
This saves over 80% of compilation time when using gccgo to compile
cmd/internal/obj/x86, as the GCC middle-end spends a lot of time
pointlessly computing interactions between temporary variables.

For golang/go#46600

Change-Id: I6f01a3c84762b9d06268c5304af6ffce14a0a031
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/331513
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
nstester pushed a commit to nstester/gcc that referenced this issue Jun 29, 2021
For a composite literal we only need to introduce a temporary variable
if we may be converting to an interface type, so only do it then.
This saves over 80% of compilation time when using gccgo to compile
cmd/internal/obj/x86, as the GCC middle-end spends a lot of time
pointlessly computing interactions between temporary variables.

For PR debug/101064
For golang/go#46600

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/331513
kraj pushed a commit to kraj/gcc that referenced this issue Jun 29, 2021
For a composite literal we only need to introduce a temporary variable
if we may be converting to an interface type, so only do it then.
This saves over 80% of compilation time when using gccgo to compile
cmd/internal/obj/x86, as the GCC middle-end spends a lot of time
pointlessly computing interactions between temporary variables.

For PR debug/101064
For golang/go#46600

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/331513
@ianlancetaylor
Copy link
Contributor

I've committed GCC patches to make this significantly faster on GCC mainline and GCC 11 branch. However, it will of course take a while for these fixes to move through the GCC release process and the distro release process.

I don't know that we really want to change the generated code merely to make gccgo builds faster. But if we do, I think that compile time would be shorter if we avoid a large composite literal and instead set each element separately, as in

	avxOptab[0] = Optab{as: AANDNL, ytab: _yandnl, prefix: Pavx, op: opBytes{
		avxEscape | vex128 | vex0F38 | vexW0, 0xF2,
	}}
	avxOptab[1] = Optab{{as: AANDNQ, ytab: _yandnl, prefix: Pavx, op: opBytes{
		avxEscape | vex128 | vex0F38 | vexW1, 0xF2,
	}}
 	...

(but I haven't actually tried it).

@mdempsky
Copy link
Contributor Author

I've committed GCC patches to make this significantly faster on GCC mainline and GCC 11 branch.

Thanks!

I think that compile time would be shorter if we avoid a large composite literal and instead set each element separately

Yeah, that seems too invasive to be worth the trouble. I assume the issue will go away in ~6 months when I upgrade to Fedora 35 and get the new GCC.

I'm happy to consider this issue fixed, unless you have more gccgo changes in mind.

@ianlancetaylor
Copy link
Contributor

Nothing else planned, so closing.

@golang golang locked and limited conversation to collaborators Jun 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants