xtensa: update to espressif/llvm-project #2533
Conversation
github-actions
bot
added
Auto-Sync-files
There was a problem hiding this comment.
What a great update! Excellent!
Only small nit picks.
Also, please seriously consider to add the instruction formats as well.
You can easily add them just like PPC does:
- Copy function: https://github.com/capstone-engine/llvm-capstone/blob/48774db25fbb9d241bf8ec1d6d3d00be61726c07/llvm/utils/TableGen/PrinterCapstone.cpp#L2714-L2744
- Base instruction class seems to be
XtensaInst\d\d(depending on bit width. So the class before theXtensaInst\d\dis the format class) (https://github.com/espressif/llvm-project/blob/2db67a2fd77dc134f025cbcd79812762877c44f1/llvm/lib/Target/Xtensa/XtensaInstrFormats.td#L12C7-L26) - Maybe even OR the the instruction width into the format enum:
enum {
XTENSA_FORMAT_INVALID = 0,
XTENSA_FORMAT_RRR_INST,
XTENSA_FORMAT_16BIT = 1 << 16,
XTENSA_FORMAT_24BIT = 1 << 17,
...
}This feature is very useful. Because if Capstone decoes stuff incorrectly and users are in a rush, they can always extract operand bits manually. So it provides an easy way of hot fixing stuff and checking for bits.
| } | ||
| } | ||
| } | ||
| printInstruction(MI, Address, O); |
There was a problem hiding this comment.
It i probably a good idea to support also alias with printAliasInstr.
But you can also skip it, if you are in a rush. Just please open an issue about it.
There was a problem hiding this comment.
I don't feel like it's a particularly important feature, and I think it could be supported later.
There was a problem hiding this comment.
Sure, no problem. Please open an issue about it after merge.
- Delete unused code - Add billow's copyright - Add test for LLVM_DEBUG patch
|
In case you missed it: capstone-engine/llvm-capstone#62 |
There was a problem hiding this comment.
Nice, almost done. Please just add a detail test case for the: format, mem_base, mem_disp, immediate and access information.
Then we are good to go. Maybe we even get it into the patch release.
@kabeor Please review as well.
This issue has been fixed in the esp branch |
Your checklist for this pull request
Detailed description
https://github.com/espressif/llvm-project
for llvm-capstone sync with this PR capstone-engine/llvm-capstone#65
Test plan
...
Closing issues
...