Skip to content

Clean up references to old opcodes #20588

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

Merged

Conversation

a7ehuo
Copy link
Contributor

@a7ehuo a7ehuo commented Nov 13, 2024

The old opcodes had the letter i as a prefix if the operation was indirect. Those opcodes were renamed to use the letter i as a suffix instead.

This commit cleans up the references to the old opcodes from isload/isstore to sloadi/sstorei

Related: #19489

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Nov 13, 2024

@hzongaro May I ask you to review this change? Thank you!

Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick grep through the code, and I didn't spot any other references to isstore or isload, except those that are actually describing whether something is a store or is a load. Thank you for cleaning up those remaining uses!

May I ask you to correct the commit comment? It mentions idload, idstore, ifload and ifstore, but none of them actually appear in this commit.

The old opcodes had the letter i as a prefix if the
operation was indirect. Those opcodes were renamed
to use the letter i as a suffix instead.

This commit cleans up the references to the old opcodes,
from isload/isstore to sloadi/sstorei

Related: eclipse-openj9#19489
Signed-off-by: Annabelle Huo <[email protected]>
@a7ehuo a7ehuo force-pushed the techdebt-cleanup-opcode-comments-5 branch from 19a53ff to cec7421 Compare November 13, 2024 21:44
@a7ehuo
Copy link
Contributor Author

a7ehuo commented Nov 13, 2024

May I ask you to correct the commit comment? It mentions idload, idstore, ifload and ifstore, but none of them actually appear in this commit.

Whoops, I just reused the commit comments from the OMR one. Updated the commit message in cec7421. Ready for another review. Thanks!

Copy link
Member

@hzongaro hzongaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@hzongaro
Copy link
Member

Jenkins test sanity.functional xlinux jdk17

@hzongaro
Copy link
Member

Looks like testing might have timed out. Rerunning. . . .

Jenkins test sanity.functional xlinux jdk17

@hzongaro hzongaro merged commit 30601f3 into eclipse-openj9:master Nov 15, 2024
5 checks passed
@a7ehuo a7ehuo deleted the techdebt-cleanup-opcode-comments-5 branch March 13, 2025 19:56
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.

2 participants