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/compile: go:readonly comment directive for body-less functions #67364

Open
Jorropo opened this issue May 14, 2024 · 4 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal
Milestone

Comments

@Jorropo
Copy link
Member

Jorropo commented May 14, 2024

Proposal Details

Add go:readonly comment directive for body-less functions indicating the following arguments and any pointers exclusively accessed through a readonly argument wont be modified:

// writeBlocks will update vector (read-hash-write) and read extra and bytes.
//
//go:nescape
//go:readonly extra, bytes
func writeBlocks(vector *[4]uint64, extra *[32]byte, bytes []byte)

The point of this is to hook up with 925d2fb this would allow []byte(str) conversions to be elided when the byte string is passed as a readonly argument to an assembly optimized subroutine.

github.com/cespare/xxhash implements Sum64String(string) uint64 and (*Digest).WriteString(string) using unsafe which allows to skip the allocation either by using (*strings.Reader).WriteTo or io.WriteString.
Other hashing libraries such as crypto do not implement this optimization, this could allow to implement this using compiler optimizations rather than duplicating APIs for []byte and string. (*note: this assume you are using a devirtualized hash.Hash or a concrete hasher implementation, which is now possible and not unrealistic, particularly for h := sha256.New(); h.Write([]byte(str)) patterns)

In the future this could improve register allocation in some rare edge cases so I don't expect this to be implemented.
Other stronger compilers like GCC and LLVM might already implement such logic in their register allocators, then work in gofrontend could be much easier (as I assume this would amount to setting up metadata flags on the function call nodes passed to gcc or llvm) so who knows.


The body-less function is allowed to read from memory, which is arguably a side effect when using -race altho I don't think very many body-less functions implement any kind of race tracking.


For slices the body-less function would assume to not be allowed to write past capacity.
If some memory is accessible through an unknown and a readonly pointer the function is allowed to write to it:

//go:readonly b
func doSomething(a, b []byte)

func f() {
 var x [64]byte
 doSomething(x[:32:32], x[:])
}

Here the compiler would be allowed to assume only the last 32 bytes of the x array are readonly (I expect compilers to not implement this or to assume x maybe completely overwritten).


If a pointer to a pointer is readonly then both memory are readonly:

type s struct{
 v *int
}

//go:readonly a
func doSomething(a *s)

func f() {
 var x int // here the compiler is allowed to assume `x` will never be modified by `doSomething`
 doSomething(&S{&x})
}

Note: this apply transitively to any amount of level.


Combining the two edge cases above, if a pointer to a pointer is accessible both through unknown and readonly paths, the body-less function may modify it:

type s struct{
 v *int
}

//go:readonly b
func doSomething(a, b *s)

func f() {
 var x int // x maybe modified through a
 doSomething(&S{&x}, &S{&x})
}

The body-less function is assumed to maybe write to anything part of the global scope:

var x int

//go:readonly a
func doSomething(a *int)

func f() {
 doSomething(&x) // doSomething is allowed to modify x
}
@gopherbot gopherbot added this to the Proposal milestone May 14, 2024
@Jorropo Jorropo moved this to Incoming in Proposals May 14, 2024
@mauri870
Copy link
Member

mauri870 commented May 14, 2024

AFAIK go directives are not part of the language spec (https://go.dev/ref/spec), it is an internal detail of the go compiler.

Go directives are generally made available for use inside internal Go packages and not user facing code, although they ended up leaking out and lurking into many codebases causing some trouble (looking at you go:linkname).

This proposal seems to aim in providing a general mechanism to optimize Go code, I'm not sure if a directive is the proper way to achieve that.

@Jorropo
Copy link
Member Author

Jorropo commented May 14, 2024

@mauri870 go:noescape has a similar goal and already exists.
I'm not doing a language spec proposal I can submit CL and debate there if that what should be done.

@bjorndm
Copy link

bjorndm commented May 15, 2024

This would be useful for functions with a body as well, and for those the compiler can check the read-only property. Although this is starting to feel like const in C..

@Jorropo
Copy link
Member Author

Jorropo commented May 15, 2024

@bjorndm this would be a different proposal, the compiler can already read body-full functions and recover the readonlyness of arguments passed in.

@cherrymui cherrymui added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 20, 2024
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. Proposal
Projects
Status: No status
Status: Incoming
Development

No branches or pull requests

5 participants