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: better optimization of type switches in instantiated generic functions #59591

Open
ianlancetaylor opened this issue Apr 13, 2023 · 2 comments
Assignees
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

@ianlancetaylor
Copy link
Contributor

Consider this test case:

package main

import (
	"fmt"
	"unsafe"
)

func start[S []byte | string](s S) *byte {
	switch s := any(s).(type) {
	case []byte:
		return unsafe.SliceData(s)
	case string:
		return unsafe.StringData(s)
	default:
		panic("can't happen")
	}
}

func F[S []byte | string](s S) {
	fmt.Println(start(s), len(s))
}

func main() {
	F("hi")
	F([]byte("bye"))
}

This runs fine. It instantiates F twice: main.F[go.shape.[]uint8] and main.F[go.shape.string]. Both versions take a dictionary, and both versions implement the type switch by loading the type descriptor from the dictionary and comparing it to the descriptors for []byte and string.

In these instantiations, however, the type descriptor is a constant. We know this because the constraint does not use a ~. It would be nice if the compiler were able to constant fold the dictionary value and eliminate the type switch.

I came up with this example because of #38776. It would be nice if we could change some of the hash implementations to use generic functions, which largely works until they try to call assembly code. At that point it's necessary to get the pointer to the slice or string data. It would be nice to be able to do that reasonably safely, which is what the above code does, and also efficiently, which the above code is not.

CC @randall77 @mdempsky

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 13, 2023
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 13, 2023
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Apr 13, 2023
@randall77
Copy link
Contributor

Note that this might be handled by my suggestion over at #48849, that if F and start inline, then we can see the load from a constant dictionary and can replace that load with the constant value we know is in that dictionary. (It would not help if inlining doesn't happen.)

This would require that all the |-separated types have a different shape. Or at least, it would only apply to shapes which correspond to just a single entry in the | list.

I think we might want to go further, doing unshaped instantiations where there is a non-~ type listed in the constraint. It would help not just with this issue (type switches), but with devirtualizing method invocations as well.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/486895 mentions this issue: cmd/compile: constant-fold loads from constant dictionaries and types

gopherbot pushed a commit that referenced this issue Apr 27, 2023
Update #59591

Change-Id: Id250a7779c5b53776fff73f3e678fec54d92a8e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/486895
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
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
Status: In Progress
Development

No branches or pull requests

4 participants