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

cmd/go: provide build tags for architecture environment variables #45454

Closed
mdempsky opened this issue Apr 8, 2021 · 9 comments
Closed

cmd/go: provide build tags for architecture environment variables #45454

mdempsky opened this issue Apr 8, 2021 · 9 comments

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Apr 8, 2021

This proposal is to have cmd/go automatically set build tags when architecture environment variables are specified.

If an architecture option GO[arch]=[option] is enabled, then cmd/go should set the [arch].[option] build tag. For example, if GOARM=6 is enabled, then cmd/go should set the "arm.6" build tag; if GOARM=7 is enabled, then it should set both the "arm.6" and "arm.7" build tags (since GOARM=7 is a superset of GOARM=6).

Precedent: We already set goexperiment.foo for each GOEXPERIMENT=foo option that's set.

Questions:

  • What to do about default flags? Do we need/want to still set "arm.5", even though that's always implied?

  • GOARM uses just "5", "6", "7", but GOPPC64 uses "power8", "power9". Should cmd/go use "ppc64.power9"? Or should it be "ppc64le.power9" for GOARCH=ppc64le?

  • What about hardfloat/sse2 vs softfloat? Is there any reason to provide 386.sse2, or should users just check for !386.softfloat? In fact, since softfloat exists across architectures, maybe a common "softfloat" tag should be used instead or in addition?

@gopherbot gopherbot added this to the Proposal milestone Apr 8, 2021
@bcmills
Copy link
Contributor

bcmills commented Apr 8, 2021

What to do about default flags? Do we need/want to still set "arm.5", even though that's always implied?

Is it possible that we will add subset options in the future? (For example, something intermediate between AMD64 v1 and v2?) If so, then I think we should set a build tag for the default, so that carefully-written code can fall back to a platform-independent implementation if compiled on a subset smaller than that default (such as in the case of the 386 softfloat transition).

@mdempsky
Copy link
Contributor Author

mdempsky commented Apr 8, 2021

Is it possible that we will add subset options in the future? (For example, something intermediate between AMD64 v1 and v2?)

I'm hoping we don't have to, but I wouldn't want to rule it out yet either.

@rsc
Copy link
Contributor

rsc commented Apr 28, 2021

GOWASM established the pattern that this environment variable can be a comma-separated list.
We were also thinking about this for GO386 when we introduced "387" etc. If there were another axis, we'd do comma-separated. (For RISC-V, we would especially need the comma-separated list for the extensions.)

So we need to figure out how a list maps. Presumably each item in the list becomes a separate "arch.thing" tag.

We also need to figure out inclusion. Presumably if you write code with a special case for //go:build amd64.v2 and then the next day v3 comes out, you want that code to still work. So these should be matched by "that setting and all future ones that imply it", same as Go versions (the go1.11 tag is set in Go 1.12 and later as well as Go 1.11). In particular, arm.5 would be true even if we are building with GOARM=6. You'd have to say arm.5 && !arm.6 to get 5-only. (It's nice to have real boolean expressions!)

It doesn't seem like we should apply any special shortenings like ppc64le.9. Let's just use what the environment variable syntax is. (ppc64le.power9 makes sense for ppc64le anywaybecause they call the architecture "POWER9".)

@rsc
Copy link
Contributor

rsc commented Apr 28, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented May 5, 2021

Will leave in the minutes for another week before moving to likely accept, but it seems like it's headed there. Last call for objections.

@rsc
Copy link
Contributor

rsc commented May 12, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented May 19, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: cmd/go: provide build tags for architecture environment variables cmd/go: provide build tags for architecture environment variables May 19, 2021
@rsc rsc modified the milestones: Proposal, Backlog May 19, 2021
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/421434 mentions this issue: go/build: add GO$GOARCH-based ToolTags

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Implement proposal golang#45454, providing build tags based on the
sub-architecture information in the GO$GOARCH variable
(for example, GOARM for GOARCH=arm).

For example, when GOAMD64=v2, the additional build tags
amd64.v1 and amd64.v2 are defined to be true.

Fixes golang#45454.

Change-Id: I7be56060d47fc61843b97fd8a78498e8202c1ee7
Reviewed-on: https://go-review.googlesource.com/c/go/+/421434
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/422615 mentions this issue: cmd/go: make cfg.BuildContext.ToolTags same order with build.Default.ToolTags

@dmitshur dmitshur modified the milestones: Backlog, Go1.20 Aug 24, 2022
gopherbot pushed a commit that referenced this issue Sep 5, 2022
…ToolTags

So it's consistent when running "go list -f '{{context.ToolTags}}'" and
printing the content of "build.Default.ToolTags".

Updates #45454

Change-Id: I7a3cbf3cdf9a6ce2b8c89e9bcf5fc5e9086d48e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/422615
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
@rsc rsc removed this from Proposals Aug 9, 2023
@golang golang locked and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants