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: deduplicate alg and noalg type descriptors to allow binary space optimizations #47904

Open
martisch opened this issue Aug 23, 2021 · 1 comment
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@martisch
Copy link
Contributor

The compiler supports SetNoAlg on types. This brings binary space savings by not needing to generate equality functions for those types. Applying this setting to backing arrays of slices (#32595) and maps (#47068) has introduced bugs into go binaries in the past because values of types that should be equal didnt compare equal as internally they were different types.

The general fix and even better space saving impact would be to have the compiler/linker deduplicate noalg with alg type descriptors giving preference to alg descriptors that link algorithms. See discussion with @mdempsky #47068 (comment)

Alternatively the compiler and linker combination could be smart enough to understand if algs can ever be used because the type is never exposed directly to user code. While this is more general and automatic, it would be more complex and require more work during compilation/linking instead of annotating the places where it is the case manually using SetNoAlg in the compiler.

Once this is ready we can save binary space by:

/cc @mdempsky @cherrymui @randall77

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/344349 mentions this issue: cmd/compile: mark type of pointer to noalg type noalg too

gopherbot pushed a commit that referenced this issue Aug 23, 2021
Arrays marked noalg are created by the compiler to hold keys and values
to initialize map literals. The ssa backend creates a pointer type for
the array type when creating an OpAddr while processing the loop that
initializes the map from the arrays. The pointer type does not inherit
the noalg property but points to the noalg array type.

This causes values created through reflect of types that should be
equal to compare unequal because the noalg and alg type might be
compared and these are not the same.

A similar problem occurred in #32595 for argument arrays of defer structs.

Created #47904 to track improve noalg handling to be able to
reintroduce this optimization again.

Fixes #47068

Change-Id: I87549342bd404b98d71a3c0f33e3c169e9d4efc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/344349
Trust: Martin Möhrmann <martin@golang.org>
Run-TryBot: Martin Möhrmann <martin@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@toothrot toothrot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 24, 2021
@toothrot toothrot added this to the Backlog milestone Aug 24, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Status: Triage Backlog
Development

No branches or pull requests

3 participants