-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add constant table to AArch64 to replace mov sequences #2464
Comments
Are you talking about doing this for native images or for code that is installed into OpenJDK? In the latter case I think you need to discuss this with @theRealAph.
I believe the plan was actually not to do what you are suggesting. That mirrors the choice made for the OpenJDK compilers. |
I'm thinking about both here.
Hmm, interesting. Do you know why OpenJDK decided on that? Note, that in A72's (and maybe prior, I'm not sure), the mov sequences will be fused [1], but that's no longer necessarily the case in new cores. It's possible this is more important for things like Graviton2. [1] https://static.docs.arm.com/uan0016/a/cortex_a72_software_optimization_guide_external.pdf section 4.11 |
@christianwimmer, is this request still valid considering the current state of the native image? |
@teshull ? |
Closing this ticket due to the lack of response. Feel free to reopen this ticket If the problem persists. |
Looking at the AArch64 assembly, it's full of mov sequences to load constants, for example:
Ideally these constants should be placed in a constants table and loaded with a single ldr instruction (plus the initial adrp instruction to load the table base).
Code patching should be updated to patch the table contents.
SPARC already has this - see LoadConstantFromTable in SPARCMoveFactory.
AArch64 has a commented out reference a LoadConstantFromTable, but no other code, so looks like there may have once been a plan to add this (... or it was just copied from SPARC).
Anything using const2reg should be patched this way. As a first pass maybe just do constants not marked as needsImmAnnotation and non zero (as zero constants are usually patched up later too - might be useful to make this more obvious in the code). However, that's a very reduced subset, so may show no obvious gains.
Small constants can still use a mov.
Need to be careful to make sure the loads don't result in tlb and cache misses.
The text was updated successfully, but these errors were encountered: