Fix vector shift opcodes on x86 - #8101
Conversation
|
Can your commit message be more specific on what was "fixed" please? Is this simply a source code formatting change? Also, please say "x86" rather than "Intel" in your wording as this isn't an Intel-specific fix. |
It's not just a formatting change. It fixes eclipse-openj9/openj9#23195 The fix is hard to see because the table was unreadable. I will update the message. |
daeba24 to
d69af46
Compare
d69af46 to
d3ef0b9
Compare
|
Addressed the comments. |
|
The comment is difficult to parse. I suggest something like: In the future, it would be better to separate the functional change (correcting the opcodes) from the code formatting change into separate commits. It was extremely tedious to isolate the subtle functional changes made in this PR using either GitHub or offline diff tools. |
It was also extremely tedious to find the bug. Only a single entry in the table caused the test failure. To identify all the typos (six so far), the table had to be reformatted. As a result, formatting and fixes became interleaved. Even if the formatting had been done separately, the diff would still have been difficult to decipher, since the original input was unreadable to begin with. On the other hand, there are offline tools that handle this case quite well. |
- Correct the entries for BinaryLogicalShiftRight and BinaryArithmeticShiftRight opcodes in the`VectorBinaryArithmeticOpCodesForMem` table to use SRL and SRA vector instructions - Format the tables for human readability
d3ef0b9 to
a74d0c3
Compare
|
Addressed comments. |
opcodes in the
VectorBinaryArithmeticOpCodesForMemtable to use SRL andSRA vector instructions