[TableGen] Fixes for per-HwMode decoding problem. #82201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today, if any instruction uses EncodingInfos/EncodingByHwMode to override the default encoding, the opcode field of the decoder table is generated incorrectly. This causes failed disassemblies and other problems. Specifically, the main correctness issue is that the EncodingID is inadvertently stored in the table rather than the actual opcode. This is caused by having set up the IndexOfInstruction map incorrectly during the loop to populate NumberedEncodings-- which is then propagated around when OpcMap is set up with a bad EncodingIDAndOpcode. Instead, do away with IndexOfInstruction altogether and use opcode value queried from CodeGenTarget::getInstrIntValue to set up OpcMap. This itself exposed another problem where emitTable was using the decoded opcode to index into NumberedEncodings. Instead pass in the EncodingIDAndOpcode vector, and create the reverse mapping from Opcode to EncodingID, which is then used to index NumberedEncodings. This problem is not currently exposed upstream since no in-tree targets yet use the per-HwMode feature. It does show up in at least two downstream targets.
|
I hope one of @topperc @jyknight @jmolloy @0x59616e @wangpc-pp (based on recent commits or past reviews to the same ageneral rea) might review this small patch, thank you. |
Could I trouble you to merge on my behalf? Thank you. |
wangpc-pp
approved these changes
Feb 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today, if any instruction uses EncodingInfos/EncodingByHwMode to override the default encoding, the opcode field of the decoder table is generated incorrectly. This causes failed disassemblies and other problems.
Specifically, the main correctness issue is that the EncodingID is inadvertently stored in the table rather than the actual opcode. This is caused by having set up the IndexOfInstruction map incorrectly during the loop to populate NumberedEncodings-- which is then propagated around when OpcMap is set up with a bad EncodingIDAndOpcode.
Instead, do away with IndexOfInstruction altogether and use opcode value queried from CodeGenTarget::getInstrIntValue to set up OpcMap. This itself exposed another problem where emitTable was using the decoded opcode to index into NumberedEncodings. Instead pass in the EncodingIDAndOpcode vector, and create the reverse mapping from Opcode to EncodingID, which is then used to index NumberedEncodings.
This problem is not currently exposed upstream since no in-tree targets yet use the per-HwMode feature. It does show up in at least two downstream targets.