Skip to content

proposal: go/build: pass test build tag #64356

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

Closed
Jorropo opened this issue Nov 23, 2023 · 2 comments
Closed

proposal: go/build: pass test build tag #64356

Jorropo opened this issue Nov 23, 2023 · 2 comments

Comments

@Jorropo
Copy link
Member

Jorropo commented Nov 23, 2023

Problem

I am trying to add pruning of known superseded implementations based on micro-architecture levels, the idea is that if you have a piece of code with:

  • A generic Go impl.
  • An SSE impl.
  • An AVX2 impl.

You usually use the cpu package to check cpuid bits.
And if I build my program with GOAMD64=v3, I don't need to link the generic Go or SSE impl, I can also remove the runtime check when calling this code because the program will then refuse to run an a cpu which don't support the features I require.

This can be implemented today using lots of const, go:build amd64/v3 and a fair dose of dark magic (small-inlinable-functions).

The problem is that this is incompatible with the way most crypto tests are implemented (in and outside of the std).
They usually have a pattern like that:

// non tests files
var useAVX2 = cpu.RequiredCPUIDBit && cpu.RequiredCPUIDBit2
var useSSE = cpu.RequiredCPUIDBit

func doThingy(args inputType) outputType {
 if useAVX2 { return avx2Impl(args) }
 if useSSE { return sseImpl(args) }
 return genericImpl(args)
}

Then in the test files they do something along the lines of:

if useAVX2 { runEndToEndTests(t); useAVX2 = false }
if useSSE { runEndToEndTests(t); useSSE = false }
runEndToEndTests(t) // generic

This allows to test all implementations on a CPU which supports AVX2.

However then I have inconciliable needs, I want production / consumers to rely on static const decision when possible, and have the test suite precisely not do that.


Proposed Solution

Make go test pass an implicit test tag.

How it solves the problem

(Note for readers, I have to use functions instead of const because they might not all be compile time known, for example when building GOAMD64=v1 this is a global variable load, while GOAMD64=v3 this can be a constant. Small-inlinable-functions achieve boths automagically without duplicating files by the hundred throughout the codebase.)

Then I can make this (all of theses 3~4 files together achieve the feature I described above):

// This could technically be folded in _test.go file today.

//go:build test
package foo

var turnOnAvx2 = avx2Compatible()
var turnSse = sseCompatible()

func useAvx2() bool {
 return turnOnAvx2
}
func useSse() bool {
 return turnOnSse
}
// I have not found a way to implement this today.

//go:build !test
package foo

func useAvx2() bool {
 return avx2Compatible()
}
func useSse() bool {
 return sseCompatible()
}
// Rest as usual.

package foo

import "internal/cpu"

func avx2Compatible() bool {
 return cpu.ExampleRequiredCpuidBit() && cpu.ExampleRequiredCpuidBit2()
}
func sseCompatible() bool {
 return cpu.ExampleRequiredCpuidBit()
}

func doThingy(args inputType) outputType {
 if useAvx2() { return avx2Impl(args) }
 if useSse() { return sseImpl(args) }
 return genericImpl(args)
}

Alternatives

  • make a testing.Test const bool (or somewhere else) which is true if invoked by go test and false otherwise, I would then still have the var turnOnXXX in the production code, however I could add a constant time shortcut inside the useXXX function, which solves the same thing.
    I'm neither positive or negative on this vs a go:build test, go:build test could solve more cases than this very narow problem tho.
  • Remove the global private feature flags and let tests call XXXImpl directly. Production always use the small-inlinable-function trick.
    This works but is undesirable and a deal breaker, many pieces of code have wrapper glue code between the consumer API and the assembly implementations, we often want End to End tests from consumer API to assembly impl. Not just testing the assembly impl.
    Alternatively you could this and have the glue code takes the assembly implementation as a virtual function, however this then force things like slices or pointers to array to escape which is also undesirable.
@gopherbot gopherbot added this to the Proposal milestone Nov 23, 2023
@seankhliao
Copy link
Member

I believe this (and const testing) were both rejected in #52600
I don't see anything that would lead us to reevaluate that decision?

@Jorropo
Copy link
Member Author

Jorropo commented Nov 23, 2023

@seankhliao I was not aware of testing.Testing thx.
Would need a compiler tweak to do what I want but this does not require a proposal or api change.

@Jorropo Jorropo closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
@golang golang locked and limited conversation to collaborators Nov 22, 2024
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

3 participants