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/compile: treat bool-to-int conversion as a constant for optimization for more complex patterns #62049

Open
dsnet opened this issue Aug 15, 2023 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Aug 15, 2023

A common argument against #9367 and #45320 is that it's trivial to just write a helper function to convert bools to ints.

However, this breaks compiler optimizations related to constant propagation.

Consider the following benchmark:

type Option interface{ option() }

type flags uint64
func (flags) option() {}

func WithRed(v bool) Option {
	return flags(btoi(v) << 15)
}

func btoi(b bool) int {
	if b {
		return 1
	} else {
		return 0
	}
}

func WithGreen(v bool) Option {
	if v {
		return flags(1 << 15)
	} else {
		return flags(0)
	}
}

var sink Option

func BenchmarkRed(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		sink = WithRed(true)
	}
}

func BenchmarkGreen(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		sink = WithGreen(true)
	}
}

Both WithRed and WithGreen are semantically equivalent, but WithGreen is dramatically faster as it does not allocate.

BenchmarkRed
BenchmarkRed-24      	100000000	        10.50 ns/op	       8 B/op	       1 allocs/op
BenchmarkGreen
BenchmarkGreen-24    	1000000000	         0.4373 ns/op	       0 B/op	       0 allocs/op

Barring the acceptance of #9367 or #45320, we should make the compiler make constant propagation work across bool-to-int helpers.

@dsnet dsnet added Proposal Performance compiler/runtime Issues related to the Go compiler and/or runtime. and removed Proposal labels Aug 15, 2023
@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 15, 2023
@dmitshur dmitshur added this to the Backlog milestone Aug 15, 2023
@dmitshur
Copy link
Contributor

CC @golang/compiler.

@dr2chase
Copy link
Contributor

I think this might not be hard in SSA, as a general rule if we see a chance to convert convT64(expr(Phi(constant1, constant2))) into Phi(convT64(expr(constant1)), convT64(expr(constant2))) we should take it. Not clear how often this fires, though.

@mknyszek mknyszek changed the title cmd/compile: treat bool-to-int conversion as a constant for optimization cmd/compile: treat bool-to-int conversion as a constant for optimization for more complex patterns Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
Development

No branches or pull requests

5 participants
@mknyszek @dmitshur @dr2chase @dsnet and others