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: amd64 addressing modes are unwieldy #36468

Open
josharian opened this issue Jan 9, 2020 · 1 comment
Open

cmd/compile: amd64 addressing modes are unwieldy #36468

josharian opened this issue Jan 9, 2020 · 1 comment

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Jan 9, 2020

There are many amd64 instructions that accept a full range of memory addressing modes. It would be nice to make use of them all.

The current approach is to give each its own op, such as BTRQmodifyidx8 or ANDQloadidx1. The problem is that this generates a large number of ops and rules. To add ANDQloadidx1, we need to find all rules starting with ANDQ and add variants for ANDQloadidx1 as appropriate. And we need to add rules (say) absorbing SHLQ of the index into the scale, and ADDQ into the offset. For an example of how many rules are involved just for adding addressing modes for CMP, see https://golang.org/cl/167089. And that's a fairly simple case, because there aren't many existing rules involving CMPQ that need indexed load analogues.

Such additions have steady incremental improvements in generated code, at the cost of developer time, reviewer time, compiler binary size, and time and memory required to build the compiler (which has proved to be an ongoing problem).

I wonder whether we can find a way to factor out some of this commonality, so we can hoover up all the incremental improvements at much less cost.

One idea is to encode the scale into AuxInt, so we'd have just idx instead of idx1, idx2, idx4, idx8. We'd need helper routines to tell us whether it's ok to adjust the scale in a particular way, since not all scales are available for all bit widths, but that is doable. The great shortcoming of this idea is that idx1 ops are commutative, but the others are not. Maybe just encode the scale for 2, 4, and 8?

Another (vague) idea is to have a separate handwritten pass after lower that does nothing but addressing modes. We'd mark ops with what kind of address modes they support in which argument slots and then do all combining then. We'd still have a combinatorial number of ops, but many fewer rules.

This is actually an instance of a broader amd64 problem around having a combinatorial number of ops (base X const? X load/store/modify X addressingmode). Maybe if we structured this all appropriately, we could generate (rather than hand code) all the ops and their encodings in amd64/ssa.go and their generated rules (like we did for commutativity).

For discussion and ideas.

cc @randall77 @mvdan @martisch @cherrymui

@toothrot toothrot added this to the Backlog milestone Jan 9, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 9, 2020

Change https://golang.org/cl/214042 mentions this issue: cmd/compile: move amd64 addressing modes to a new pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.