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

Improve arraytranslateEvaluator for Power #7649

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Feb 10, 2025

This commit removes the code generation for checking length == 0 in arraytranslateEvaluator for Power.
The helper functions for arraytranslate are responsible for checking the case of length == 0.

This commit removes the code generation for checking length == 0 in
arraytranslateEvaluator for Power.
The helper functions for arraytranslate are responsible for checking
the case of length == 0.
@knn-k
Copy link
Contributor Author

knn-k commented Feb 10, 2025

The arraytranslate evaluator generates code like this without this change:

    0x7a940ffc016c 000000f8 [    0x7a940f0d95f0]                   10   Label L0097:    ; (Start of internal control flow)
    0x7a940ffc016c 000000f8 [    0x7a940f0d9680] 2c250000          10   cmpdi   cr0, gr5, 0
    0x7a940ffc0170 000000fc [    0x7a940f0d9720] 4181000c          10   bgt     cr0, Label L0098
    0x7a940ffc0174 00000100 [    0x7a940f0d97c0] 38600000          10   li      gr3, 0000000000000000
    0x7a940ffc0178 00000104 [    0x7a940f0d9860] 48000018          10   b       Label L0099
    0x7a940ffc017c 00000108 [    0x7a940f0d98f0]                   10   Label L0098:
    0x7a940ffc017c 00000108 [    0x7a940f0dc9a0] 60600000          10   ori     gr0, gr3, 0x0
    0x7a940ffc0180 0000010c [    0x7a940f0d9a10] 481ffba1          10   bl      00007A94101BFD20 ; Direct Call "__arrayTranslateTROT255 (ISO8859_1)"
    0x7a940ffc0184 00000110 [    0x7a940f0dc900] 7c630278          10   xor     gr3, gr3, gr0
    0x7a940ffc0188 00000114 [    0x7a940f0dc860] 7c600278          10   xor     gr0, gr3, gr0
    0x7a940ffc018c 00000118 [    0x7a940f0dc720] 60030000          10   ori     gr3, gr0, 0x0
    0x7a940ffc0190 0000011c [    0x7a940f0d9b10]                   10   Label L0099:    ; (End of internal control flow)

This commit removes the cmpi and the bgt from the main line of the generated code, in addition to eliminating the inappropriate register dependency at labelArrayTranslateDone that caused annoying register shuffles.
All the arraytranslate helper functions, except for TR_PPCarrayTranslateTRTO, work as expected when the length is zero. A PR in OpenJ9 changes the TRTO helper to handle the case of length zero.

The performance degrades with this change when the length is zero, but I assume that does not happen very often.

@knn-k
Copy link
Contributor Author

knn-k commented Feb 10, 2025

Jenkins build plinux,aix

@knn-k
Copy link
Contributor Author

knn-k commented Feb 10, 2025

There is no need for a coordinated merge. eclipse-openj9/openj9#21087 is to be merged first, and this PR to follow.

The test failure on plinux is a known problem: Issue #6571:

[2025-02-10T02:18:02.862Z] 31: [0;32m[----------] [m16 tests from Special/PPCDirectEncodingTest
[2025-02-10T02:18:02.862Z] 31: free(): invalid next size (normal)

@knn-k
Copy link
Contributor Author

knn-k commented Feb 12, 2025

@zl-wang FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant