Skip to content

proposal: cmd/compile: go:readonly comment directive for body-less functions #67364

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

Open
Jorropo opened this issue May 14, 2024 · 5 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

This comment was marked as off-topic.

@Jorropo

This comment was marked as off-topic.

@cherrymui cherrymui added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 20, 2024
@Jorropo
Copy link
Member Author

Jorropo commented Dec 24, 2024

I just wrote https://go-review.googlesource.com/c/go/+/637936 and I think it is valuable to allow bodyless functions to say they wont read or write to global state.
I'm not sure how it should look tho.
I can think of //go:readonly * for disallowing any observable writes but this is inflexible (doesn't allow to treat global state as readonly but not some of the arguments).

@seankhliao seankhliao added the GoCommand cmd/go label Feb 22, 2025
@matloob matloob removed the GoCommand cmd/go label Mar 24, 2025
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

7 participants