Clean up references to old opcodes #7519
Conversation
github-actions
bot
added
arch:aarch32
arch:aarch64
arch:power
arch:x86
arch:z
comp:compiler
labels
|
@hzongaro May I ask you to review this change? Thank you very much! This is the first set of change that cleans up the old references in the comments or traces. I'll have another change that cleans up the old references in the code itself. |
|
Jenkins build all |
There was a problem hiding this comment.
I think the changes look good, and it looks like you've caught all of the cases where the old opcode names appeared in comments and trace messages.
May I ask you to adjust the commit comment to reduce the length of the first line to less than 70 characters, and also mention in the commit comment the reason for the change?
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, iiload/iistore ilload/ilstore iaload/iastore ibload/ibstore, to iloadi/istorei lloadi/lstorei aloadi/astorei bloadi/bstorei in the code comments and trace messages. Related: eclipse-openj9/openj9#19489 Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo
force-pushed
the
techdebt-cleanup-opcode-comments
branch
from
2b87ea2 to
6e681da
Compare
a7ehuo
changed the title
There was a problem hiding this comment.
I just realized that this change handles references to indirect loads and stores for 8-bit, 32-bit and 64-bit integer values, but not 16-bit — that is, isload and isstore should become sloadi and sstorei respectively. I've pointed out a couple of examples. Did you intend to handle those in this pull request or were you planning on opening a separate pull request for them? Of course you'll have to distinguish cases where code is speaking of whether something is a load or store from cases where it's talking about indirect short load and store operations.
Also, I noticed references to idload and idstore in comments in VPHandlers.cpp
I plan to handle the rest of the opcodes, isload/isstore, ifload/ifstore, idload/idstore, in future pull requests. This clean up work touches a lot of files and there will be dependencies between OMR and OpenJ9 as well when it comes to updating the code itself. Breaking them up in multiple PRs makes the test and review easier. My plan is Step 1. Clean up iiload/iistore ilload/ilstore iaload/iastore ibload/ibstore in the code comments and trace messages in OMR |
|
The only change since the previous build was run was in the commit comments. That build was successful, so merging. |
The old opcodes had the letter i as a prefix if the operation was indirect. Those opcodes were renamed to use the letter as a suffix instead.
This commit cleans up the references to the old opcodes, iiload/iistore ilload/ilstore iaload/iastore ibload/ibstore, to iloadi/istorei lloadi/lstorei aloadi/astorei bloadi/bstorei in the code comments and trace messages.
Related: eclipse-openj9/openj9#19489