-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Architecture updater (auto-sync) - Updating PPC #2013
Conversation
Hi @terorie! As you see I am just in the process to refactor the PPC module. You added the |
Hey @Rot127 unfortunately I made minimal progress here. There was a TableGen version mismatch between DarkKirb's patch and the one that Capstone needed. So, I started learning and rewriting TableGen rules by hand a long time ago but not sure where I left that off. Will share the code if I can find it again. Happy to pick this back up if there's interest. |
This would be great!
While I am at it I would try to add it on my own. But can I come back to your offer if I get stuck? The priority is to focus on refactoring as many archs as possible to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor things
@Rot127 no objections against asserts if they are disabled by default. Maybe could be something like |
@terorie The Details
|
Rebased and squashed this PR onto the latest commit of #1949 (683a595). |
@Rot127 some fuzzer job bugs found - check the CI: https://github.com/capstone-engine/capstone/actions/runs/5494173005/jobs/10012800208?pr=2013#step:5:82 |
Yes, I am going to fuzz it as well before starting with Rizin integration. |
Please rebase on top of latest |
This comment was marked as resolved.
This comment was marked as resolved.
@Rot127 I will start to review the code this weekend. |
Have you had a chance to look? |
Would be nice to get this finally merged. It will reduce the amount of work and will make reviewing Aarch64 and Alpha PRs much easier. |
Yeah I am reviewing the code, still need time. |
Any updates? |
Every thing looks great! Merged. |
Don't worry about red CI - it's a GitHub/network issue. Could be restarted later today:
|
CS_AC_INVALID = 0, ///< Uninitialized/invalid access type. | ||
CS_AC_READ = 1 << 0, ///< Operand read from memory or register. | ||
CS_AC_WRITE = 1 << 1, ///< Operand write to memory or register. | ||
CS_AC_READ_WRTE = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo wrte -> write
Base on: #1949
Draft PR for the
auto-sync
refactor of PPC.It will also refactor quite a lot of general mapping logic used by
auto-sync
archsTODO
not set
flag.add_cs_detail
to newcs_mapping.h
TH
operands fordcbt
td
files"0x04,0x00,0x00,0x00,0xf4,0xa4,0x00,0x08"
->pstd 5, 8(4), 0
Optimizations to do afterwards (in another PR)
;
for proper formatting.CAPSTONE_DIET
get_cs_reg_alias()
for non LLVM register names.add_cs_group()
for non LLVM instruction groups.PPCPredicate.h
to LLVMCloses
closes #1909
closes #476
closes #1936
closes #1914
closes #1912
closes #1693
closes #1527
closes #944
Test again
bne
misses target address #1914 (missing reg alias)mtocrf
,mfocrf
missesfxm
immediate #1903 (hits assert)Breaking changes
PPC_BC_NU_PLUS
->PPC_PRED_NU_PLUS
).cs_ppc.bc
.reg = r0
. This is fixed now.ppc_ops_crx
is removed (wasn't used).