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: inlining after devirtualization #52193

Closed
FiloSottile opened this issue Apr 6, 2022 · 4 comments
Closed

cmd/compile: inlining after devirtualization #52193

FiloSottile opened this issue Apr 6, 2022 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@FiloSottile
Copy link
Contributor

In #33160 we talked about an escape analysis pass after devirtualization, to prevent arguments of devirtualized interface method calls from escaping to the heap. This made it possible to use crypto/sha256.New256().Write(buf) without leaking buf. ✨

Separately, I've been using the mid-stack inliner to outline allocations into the caller, enabling allocation-free APIs that ergonomically return a []byte. 🎉

I am now designing the crypto/ecdh API, and I'd like to combine these two techniques, because the cleanest API we could come up with @rsc so far is something like this

type Curve interface {
	GenerateKey(rand io.Reader) (*PrivateKey, error)
	NewPublicKey(key []byte) (*PublicKey, error)
	ECDH(local *PrivateKey, remote *PublicKey) ([]byte, error)
	// unexported methods so can't be externally implemented
}

func P256() Curve
func P384() Curve
func P521() Curve

which you'd use like this

p256 := ecdh.P256()

ourKey, err := p256.GenerateKey(rand.Reader)
if err != nil { t.Fatal(err) }

peerPublic, err := p256.NewPublicKey(peerShare)
if err != nil { t.Fatal(err) }

sharedSecret, err := p256.ECDH(ourKey, peerPublic)
if err != nil { t.Fatal(err) }

Unfortunately, there is no inlining pass after devirtualization, so although p256.GenerateKey et al get devirtualized, they don't get inlined, and their allocations don't end up on the caller's stack.

How doable would it be to have an inlining pass after devirtualization and before escape analysis? (It doesn't have to come before crypto/ecdh, but how likely it is to come later will influence whether we go with this or with more verbose concrete return values.)

/cc @mdempsky @randall77

@FiloSottile FiloSottile added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 6, 2022
@mvdan
Copy link
Member

mvdan commented Apr 7, 2022

cc @mundaym @josharian as I seem to recall some threads years ago about repeating some of the compiler optimization passes where the improvement versus compile time tradeoff would still make sense.

@egonelbre
Copy link
Contributor

Maybe it would be possible to restrict the extra inlining pass to the funcs that contain devirtualizations to reduce the extra compile time?

@CAFxX
Copy link
Contributor

CAFxX commented Apr 7, 2022

Related: #38992

@gopherbot
Copy link

Change https://go.dev/cl/528321 mentions this issue: cmd/compile: interleave inlining and devirtualization

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. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

6 participants