Skip to content
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

i#3544 RV64: Port sample 'cbr' to RISCV64 #6837

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Conversation

chenhy0106
Copy link
Contributor

@chenhy0106 chenhy0106 commented Jun 3, 2024

  1. Conditional branch instruction of RISC-V 'C' extension may not reach after adding clean call. So like X86, we add support to detect and convert compressed cbr to longer version.
  2. For RISCV64, when a cbr instruction uses the stolen reg, we need to replace it with a scratch reg.

Now cbr works for RISCV64. Still does not work for ARM32/ARM64 because of some bugs unrelated to cbr sample.

Issue: #3544

1. For RISCV64, conditional branch instruction of 'C' extension may not reach after adding clean call. So like X86, we add support to detect and convert compressed cbr to longer version.
2. For AARCH64 and RISCV64, a cbr may use the stolen reg and can not be mangled later as it is meta. So we check whether a cbr uses the stolen reg and replace it with a scratch reg. Now cbr works for AARCH64 and RISCV64. Still not work for ARM32 because of some bugs unrelated to cbr sample.

Issue: DynamoRIO#1569, DynamoRIO#3544
api/samples/CMakeLists.txt Show resolved Hide resolved
api/samples/cbr.c Outdated Show resolved Hide resolved
api/samples/cbr.c Outdated Show resolved Hide resolved
api/samples/cbr.c Outdated Show resolved Hide resolved
api/samples/cbr.c Show resolved Hide resolved
Copy link
Contributor

@ksco ksco left a comment

Choose a reason for hiding this comment

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

Looks like AArch64 CI is failing, and the cbr test is not enabled on RISC-V CI.

@chenhy0106 chenhy0106 changed the title i#1569, i#3544 Port sample 'cbr' to AARCH64 and RISCV64. i#3544 Port sample 'cbr' to RISCV64. Jun 17, 2024
@chenhy0106 chenhy0106 changed the title i#3544 Port sample 'cbr' to RISCV64. i#3544 RV Port sample 'cbr' to RISCV64. Jun 17, 2024
@chenhy0106 chenhy0106 changed the title i#3544 RV Port sample 'cbr' to RISCV64. i#3544 RV64: Port sample 'cbr' to RISCV64. Jun 17, 2024
@chenhy0106 chenhy0106 changed the title i#3544 RV64: Port sample 'cbr' to RISCV64. i#3544 RV64: Port sample 'cbr' to RISCV64 Jun 17, 2024
@ksco ksco requested a review from derekbruening June 17, 2024 09:02
@derekbruening
Copy link
Contributor

(May not get to this for another day)

api/samples/CMakeLists.txt Outdated Show resolved Hide resolved
api/samples/CMakeLists.txt Outdated Show resolved Hide resolved
api/samples/cbr.c Outdated Show resolved Hide resolved
core/lib/instrument.c Outdated Show resolved Hide resolved
@ksco
Copy link
Contributor

ksco commented Jun 20, 2024

LGTM, merging.

@ksco ksco merged commit e43cb86 into DynamoRIO:master Jun 20, 2024
16 of 17 checks passed
@ksco
Copy link
Contributor

ksco commented Jun 20, 2024

Hi @derekbruening, @chenhy0106 is an intern in our lab, and he would like to submit more PRs to DynamoRIO on RV64 support. Can we have him in the committer team so he can merge PRs himself in the future?

@derekbruening
Copy link
Contributor

Hi @derekbruening, @chenhy0106 is an intern in our lab, and he would like to submit more PRs to DynamoRIO on RV64 support. Can we have him in the committer team so he can merge PRs himself in the future?

Done.

@chenhy0106 chenhy0106 deleted the port_cbr branch June 23, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants