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

x/build: fake CPU detection for builds? #12805

Open
bradfitz opened this Issue Oct 1, 2015 · 12 comments

Comments

Projects
None yet
9 participants
@bradfitz
Member

bradfitz commented Oct 1, 2015

Our builders for the most part only run on Haswell CPUs on Google Compute Engine.

A number of Go packages in the standard library and runtime use assembly to choose alternate paths depending on the capabilities of the processor.

I feel like we're not getting good test coverage, only testing the Haswell paths.

We do have a GOARCH=386 builder, though, which is at one extreme, but there are a number of steps in the middle untested.

Can we put an abstraction over CPUID etc somewhere (unexported) such that the builders can fake it before assembly code can?

/cc @ianlancetaylor @minux @adg

@bradfitz bradfitz added the Builders label Oct 1, 2015

@minux

This comment has been minimized.

Member

minux commented Oct 7, 2015

@randall77

This comment has been minimized.

Contributor

randall77 commented Oct 7, 2015

For the runtime at least, I don't think we can use a separate package. The
runtime needs to know very early on some of the processor flags so that it
can initialize properly (alg tables in particular, maybe other things).
The runtime package shouldn't be allowed to import any other packages.
Currently the runtime only imports unsafe (which is, paradoxically, safe to
import because it contains no code, no initializers, ...). It sounds dicey
to allow anything more substantial. How would that package get
initialized? Where would it get a stack?

That said, runtime will always be special. This proposal sounds reasonable
for use by all other packages. Maybe the implementation can be in package
runtime and runtime/cpu gets magic unexported access to runtime (ala
reflect). At that point, though, why not just add the API to runtime
directly?

On Tue, Oct 6, 2015 at 5:07 PM, Minux Ma notifications@github.com wrote:

The main problem is that a lot of packages issue their own
CPUID instruction. We can't override that without refactor
them to use a centralized cpu feature detection facility.

I'm wondering if we could have a runtime/cpu package that
provide a public interface to query CPU capabilities?
we can't just have an internal package because some of
the code in sub-repo also needs CPU feature detection.

If we have a standard interface, then 3rdparty code will also
benefit from this kind of testing without forcing to test on
a large variety of machines.

If you think this standard interface is reasonable, I can write
a proposal.

Basically, the runtime/cpu package will provide overridable
runtime cpu feature detection.


Reply to this email directly or view it on GitHub
#12805 (comment).

@adg

This comment has been minimized.

Contributor

adg commented Oct 7, 2015

On 7 October 2015 at 11:22, Keith Randall notifications@github.com wrote:

For the runtime at least, I don't think we can use a separate package. The
runtime needs to know very early on some of the processor flags so that it
can initialize properly (alg tables in particular, maybe other things).
The runtime package shouldn't be allowed to import any other packages.
Currently the runtime only imports unsafe (which is, paradoxically, safe to
import because it contains no code, no initializers, ...). It sounds dicey
to allow anything more substantial. How would that package get
initialized? Where would it get a stack?

That said, runtime will always be special. This proposal sounds reasonable
for use by all other packages. Maybe the implementation can be in package
runtime and runtime/cpu gets magic unexported access to runtime (ala
reflect). At that point, though, why not just add the API to runtime
directly?

I'd imagine the runtime/cpu package would depend on internal functions from
inside the runtime package, the same way time does.

@griesemer

This comment has been minimized.

Contributor

griesemer commented Oct 7, 2015

Presumably there's no way to configure GCE to "select" a (virtually) different CPU other than Haswell? (It would seem that this might be an issue for other clients as well, not just for us.)

@adg

This comment has been minimized.

Contributor

adg commented Oct 7, 2015

No, you just get the processor type of the host machine.

IIUC some GCE instances might have different kinds of processors, but you
can't ask for one.

On 7 October 2015 at 11:44, Robert Griesemer notifications@github.com
wrote:

Presumably there's no way to configure GCE to "select" a (virtually)
different CPU other than Haswell? (It would seem that this might be an
issue for other clients as well, not just for us.)


Reply to this email directly or view it on GitHub
#12805 (comment).

@bradfitz

This comment has been minimized.

Member

bradfitz commented Oct 7, 2015

We could run in different zones with different processors, but that is a very limited selection compared to what Go aims to support.

For testing, I'd really like to cover many more paths.

@minux

This comment has been minimized.

Member

minux commented Oct 7, 2015

@rsc rsc added this to the Unreleased milestone Oct 23, 2015

@martisch

This comment has been minimized.

Member

martisch commented Jun 4, 2017

Now that for go1.9 the std lib cpuid usage has been unified to use internal/cpu i will work on a proposal and implementation for go1.10 to extend the cpu package such that we can set cpu flags for the std lib.

If we use a standardized set of environment variables we could also augment other copies of the cpu feature detection code (e.g. in x/crypto which partly is vendored into std lib) to take these into account too and leaving internal/cpu still internal.

For the runtime we would still need build infrastructure with virtual machines where we can set the cpuid mask with the hypervisor for the virtual machine.

@quentinmit

This comment has been minimized.

Contributor

quentinmit commented Jun 4, 2017

Keep in mind what happened with ARM - we did this with ARM and invalid instructions slipped in because the processor still supported them even though the environment variable said to not use them.

The best way to test this really is machines with the old instruction sets. Or I suppose we could run inside bochs, but that would probably be horrifically slow. AFAICT qemu doesn't support controlling CPU feature flags.

@martisch

This comment has been minimized.

Member

martisch commented Jun 4, 2017

Thanks for the hint. Indeed it wont prevent using instructions that are not guarded by feature detection. It however would support testing output of code paths with the builders that are otherwise not tested since e.g. all amd64 builders support AVX and there are SSE41 code paths.

@bradfitz

This comment has been minimized.

Member

bradfitz commented May 4, 2018

Now that gvisor is open source (https://github.com/google/gvisor), we can run some build configurations under gvisor and its emulated CPUID instructions to mask away certain CPU features and get more test coverage of our code paths selected at runtime.

/cc @bcmills @andybons @FiloSottile

@gopherbot

This comment has been minimized.

gopherbot commented May 10, 2018

Change https://golang.org/cl/91737 mentions this issue: internal/cpu: use GOCPU environment variable to disable cpu features

gopherbot pushed a commit that referenced this issue May 22, 2018

internal/cpu: add experiment to disable CPU features with GODEBUGCPU
Needs the go compiler to be build with GOEXPERIMENT=debugcpu to be active.

The GODEBUGCPU environment variable can be used to disable usage of
specific processor features in the Go standard library.
This is useful for testing and benchmarking different code paths that
are guarded by internal/cpu variable checks.

Use of processor features can not be enabled through GODEBUGCPU.

To disable usage of AVX and SSE41 cpu features on GOARCH amd64 use:
GODEBUGCPU=avx=0,sse41=0

The special "all" option can be used to disable all options:
GODEBUGCPU=all=0

Updates #12805
Updates #15403

Change-Id: I699c2e6f74d98472b6fb4b1e5ffbf29b15697aab
Reviewed-on: https://go-review.googlesource.com/91737
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment