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: match Aux/AuxInt more strictly in rewriting rules #44824

Open
cherrymui opened this issue Mar 5, 2021 · 0 comments
Open

cmd/compile: match Aux/AuxInt more strictly in rewriting rules #44824

cherrymui opened this issue Mar 5, 2021 · 0 comments
Labels
Milestone

Comments

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Mar 5, 2021

Currently, in a rewriting rule, on the LHS if a value's Aux or AuxInt is not specified, it is ignored by the rule matcher. This is error-prone. One example is a rule introduced in CL https://go-review.googlesource.com/c/go/+/280456, where the ignored Aux/AuxInt causes miscompilation (#44823).

To make it less error-prone, maybe we want the rule matcher to require Aux/AuxInt to be always specified on the LHS (using _ to explicitly ignore)? Or maybe we want it to match the zero value if not specified?

Discussion from CL https://go-review.googlesource.com/c/go/+/299289:
@randall77 :

maybe we want the rule matcher require Aux/AuxInt to be always specified on the LHS

Yes please. Now with strongly typed rules it should be fairly straightforward.

Speaking of which, I was hoping at some point to get rid of the distinction between Aux and AuxInt. Now that everything is typed, we can just have Aux types for everything, and some of them might be {Sym, int32} pairs or whatever. Value.Aux and Value.AuxInt would just be the storage for those aux values.
(Which should be great for rewrite rules. I'm not sure how great/terrible it would be for other compiler passes.)

@josharian :

the value of auxint is that it avoids an alloc.

@toothrot toothrot added this to the Backlog milestone Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants