Skip to content

Add tricore tc1.8 instructions #2595

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.

Sign up for GitHub

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

Jump to bottom
Merged
kabeor merged 6 commits into from
Jan 28, 2025
Merged

Add tricore tc1.8 instructions #2595

kabeor merged 6 commits into from
Jan 28, 2025

Conversation

@Changqing-JING
Copy link
Contributor

@Changqing-JING Changqing-JING commented Jan 6, 2025 •
edited
Loading

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

Add tricore tc1.8 instructions:

add.df
sub.df
madd.df
msub.df
mul.df
div.df
cmp.df
max.df
min.df
min.f
max.f
dftoi
dftoiz
dftoin
ftoin
dftou
dftouz
dftol
dftoul
dftoulz
abs.f
abs.df
dftolz
neg.df
neg.f
qseed.df
itodf
utodf
ltodf
ultodf
dftof
ftodf
div64
div64.u
rem64
rem64.u

The inc files are updated by:
capstone-engine/llvm-capstone#73

...

Test plan

cmake --build ./build/linux-x64 && ./build/linux-x64/suite/cstest/Debug/cstest tests/MC/TriCore > build/test.lo

...

Closing issues

...

@XVilka
Copy link
Contributor

XVilka commented Jan 6, 2025

@imbillow and this one, please

cc @Rot127

@Changqing-JING
Copy link
Contributor Author

Changqing-JING commented Jan 9, 2025

Since this PR is not merged yet. I added a bit more instructions

div64
div64.u
rem64
rem64.u

Rot127
Rot127 reviewed Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to find time for a full review on the weekend. But please don't forget to add tests for the instruction details in tests/details.

@Changqing-JING
Copy link
Contributor Author

Changqing-JING commented Jan 10, 2025

@Rot127 Thank you very much for review.
I have added tests to tests/details

But I still have a question. The tests under tests/details seems similar to tests/MC. What's the reason to have both? Why not merge them to one?

@Rot127
Copy link
Collaborator

Rot127 commented Jan 10, 2025 •
edited
Loading

But I still have a question. The tests under tests/details seems similar to tests/MC. What's the reason to have both? Why not merge them to one?

The MC tests are copies of the assembler/disassembler regression tests from LLVM. They get translated from the LLVM format to our yaml test files with the Auto-Sync.
They effectively test all code which is from LLVM. And only the asm text. Not any of the details.

The detail tests on the other hand, test the code added by Capstone. So all the details. They should (but don't, yet) cover all functions in <ARCH>Mapping.c files.

Rot127
Rot127 requested changes Jan 12, 2025
View reviewed changes
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely! Thanks a lot!
Just address those small comments please. Otherwise lgtm.
@kabeor

@Changqing-JING
Copy link
Contributor Author

Changqing-JING commented Jan 15, 2025

Can we merge this PR?

b1llow
b1llow approved these changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@b1llow b1llow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Rot127
Copy link
Collaborator

Rot127 commented Jan 15, 2025 •
edited
Loading

@Changqing-JING @kabeor has to review first. I don't have the privilege to merge without a second maintainer's approval.

@Rot127
Copy link
Collaborator

Rot127 commented Jan 16, 2025

Please rebase onto the latest next branch to run the labeler again.

@Changqing-JING
Add tricore tc1.8 instructions:
fdf416f 
add.df
sub.df
madd.df
msub.df
mul.df
div.df
cmp.df
max.df
min.df
min.f
max.f
dftoi
dftoiz
dftoin
ftoin
dftou
dftouz
dftol
dftoul
dftoulz
abs.f
abs.df
dftolz
neg.df
neg.f
qseed.df
itodf
utodf
ltodf
ultodf
dftof
ftodf
@Changqing-JING
Fix python binding
a5b4a7a
@Changqing-JING
Fix python binding
78ccc5a
@Changqing-JING
add tricore tc1.8 instructions
86903b9 
div64
div64.u
rem64
rem64.u
@Changqing-JING
add tricore tc1.8 instruction to tests/details
de33251
@Changqing-JING
Fix review
b6add3f
@Changqing-JING
Copy link
Contributor Author

Changqing-JING commented Jan 17, 2025

Rebased

@Changqing-JING
Copy link
Contributor Author

Changqing-JING commented Jan 20, 2025 •
edited
Loading

@kabeor Could you help to review this PR?

kabeor
kabeor approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@kabeor kabeor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, merged. Thx!

@kabeor kabeor merged commit 3c4d7fc into capstone-engine:next Jan 28, 2025
20 checks passed
@Changqing-JING Changqing-JING deleted the feature/tricore-tc18 branch January 28, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kabeor kabeor kabeor approved these changes

@Rot127 Rot127 Rot127 approved these changes

+1 more reviewer

@b1llow b1llow b1llow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CS-core-files auto-sync LLVM-generated-files Python Bindings TriCore Arch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Changqing-JING @XVilka @Rot127 @b1llow @kabeor