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

proposal: cmd/go: add build tags for 32-bit and 64-bit architectures #33388

Open
onitake opened this issue Jul 31, 2019 · 11 comments

Comments

@onitake
Copy link

commented Jul 31, 2019

What version of Go are you using (go version)?

$ go version
go version go1.11.6 linux/amd64

Does this issue reproduce with the latest release?

Unknown

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

I'm working on a slightly specialised math utility library that exports functions similiar to math.Abs, but for various data types and with certain close-to-hardware optimisations.
Since Go makes no special conventions for the generic int and uint integer types, it is difficult to provide universal implementations for all supported GOARCHs.

It is also very hard to provide unit tests that work across architectures, leading to tests limited to specific architectures, on top of the impossibility to make them future-proof.

As an example, consider the optimised code for calculating the absolute value of an int64 described here: http://cavaliercoder.com/blog/optimized-abs-for-int64-in-go.html

func abs(n int64) int64 {
	y := n >> 63
	return (n ^ y) - y
}

Such an implementation will work well for well-defined data types like int32 and int64, but not int.

What did you expect to see?

Go should make it very easy to separate code into 32-bit architecture and 64-bit architecture code path by using some sort of generic build tag:

// +build 32bit
package imath
func abs(n int) int {
	y := n >> 63
	return (n ^ y) - y
}
// +build 64bit
package imath
func abs(n int) int {
	y := n >> 31
	return (n ^ y) - y
}

What did you see instead?

There is no such build tag, leading to ugly hacks such as using unsafe.SizeOf(int) to separate code paths or limiting optimised code to only specific GOARCHs:

import "unsafe"
var abs func (int) int
func init() {
    switch unsafe.Sizeof(int) {
    case 4:
        abs = absInt32
    case 8:
        abs = absInt64
    default:
        abs = absUnoptimised
    }
}
// +build 386 arm armbe mips mipsle ppc s390 sparc
// +build amd64 amd64p32 arm64 arm64be ppc64 ppc64le mips64 mips64le mips64p32 mips64p32le s390x sparc64

@gopherbot gopherbot added this to the Proposal milestone Jul 31, 2019

@gopherbot gopherbot added the Proposal label Jul 31, 2019

@randall77

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

We do this in the runtime:

const PtrSize = 4 << (^uintptr(0) >> 63)           // unsafe.Sizeof(uintptr(0)) but an ideal const

You could do a similar thing with int/uint and then write your abs using that constant.

const intBits = 4 + 4*(^uint(0)>>63)
func abs(n int) int {
	y := n >> (intBits - 1)
	return (n ^ y) - y
}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

For what it's worth, we can see 32-bit vs. 64-bit in runtime/lfstack_32bit.go vs. runtime/lfstack_64bit.go and in runtime/hash32.go vs. runtime/hash64.go.

@randall77

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

I'm not against this proposal, but the given example doesn't motivate me. With build tags you have to write each function twice. With appropriate constants you only have to write them once.
(Or you use the build tags to just define the constant, which seems overkill.)

@smasher164

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Also see #29982, which saves you from computing the sizes yourself.

@onitake

This comment has been minimized.

Copy link
Author

commented Aug 2, 2019

@randall77 My point here is that writing an optimised function twice might be exactly what you're after. Using a type-specific constant could be a possible solution, but might not always work.

@smasher164 That's a good suggestion. It's a bit unfortunate that the proposal wasn't considered yet, but math/bits.UintSize might be good enough for this use case. It still wouldn't address cases where clever bit twiddling and shifting by a constant solves the problem.

Let me see if I can come up with an example where constants won't work.

@randall77

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

If you have a constant you can always write two different implementations if you want. Either:

func f() {
    if intSize == 64 {
        ... implementation 1 ...
    } else {
        ... implementation 2 ...
    }
}

or

func f() {
    if intSize == 64 {
        f64()
    } else {
        f32()
    }
}
func f64() {
    ... implementation 1 ...
}
func f32() {
    ... implementation 2 ...
}

Either way if intSize is a constant the wrapping gets completely compiled away.

@onitake

This comment has been minimized.

Copy link
Author

commented Aug 3, 2019

Unfortunately, this does not apply to numeric literals:

package main
import "testing"
const intSize = 4 + (^uint(0)>>63) * 4
func TestInt(t *testing.T) {
	var v []int
	if intSize == 8 {
		v = []int{ -0x7ffffffffffffffe, 0x7ffffffffffffffe, -0x7fffffffffffffff, 0x7fffffffffffffff }
	} else {
		v = []int{ -0x7ffffffe, 0x7ffffffe, -0x7fffffff, 0x7fffffff }
	}
	t.Logf("intSize=%d v[0]=%d", intSize, v[0])
}

will produce the following, when compiled on 32bit:

./int_test.go:7:14: constant -9223372036854775806 overflows int
./int_test.go:7:35: constant 9223372036854775806 overflows int
./int_test.go:7:55: constant -9223372036854775807 overflows int
./int_test.go:7:76: constant 9223372036854775807 overflows int
@rsc

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

@onitake, the way to avoid that issue is typically to write constants like intSize that serve as masks and then mask the values. Or in this case:

const maxInt = 1<<(intSize*8-1) - 1
v := []int{-(maxInt-1), maxInt-1, -maxInt, maxInt}

It's not ideal but it doesn't come up too often and is much lower complexity to maintain than separate files with separate build tags.

@rsc rsc changed the title proposal: add build tags for 32-bit and 64-bit architectures proposal: cmd/go: add build tags for 32-bit and 64-bit architectures Aug 6, 2019

@onitake

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

🤔 Hmmmmm... I suppose I can do it this way.

I still think it's overly cryptic and counter-intuitive, though.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

Issue #28538 suggested adding math.MaxInt etc and was accepted but never implemented for Go 1.13. It is still open and can be implemented for Go 1.14. Assuming that does happen, then the solution to #33388 (comment) is to use those named constants.

Given the multiple (and usually much better) ways to write 32- or 64-bit specific code, it seems like adding build tags is heavy-weight and redundant. It seems like we should decline adding build tags based on architecture size.

Any thoughts?

@onitake

This comment has been minimized.

Copy link
Author

commented Aug 20, 2019

It might depend on the perspective, but 32/64 bit build tags don't seem particularly "heavy-weight" to me...
But I understand the reluctance to add additional features without a good reason, particularly if there is an alternative solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.