i#3544 riscv64: Implement some base instructions

[shiptux]

This commit introduces the following changes:

• Added SYSNUM_REG for RISC-V64
• Implemented atomic_add
• Implemented dynamorio_sigreturn
• Implemented dynamorio_sys_exit
• Implemented call_switch_stack

Notice: Although some base instructions have been implemented for RISC-V64, there are still many unimplemented pieces in Dynamorio for this architecture, and it cannot be tested via QEMU then.

Issue: #3544

[abhinav92003]

Also, s/instrutions/instructions/ in the PR title.

[shiptux]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches.
https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133
@derekbruening @abhinav92003

[abhinav92003]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches. https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133 @derekbruening @abhinav92003

Does seem that some of that #else part is intended only for AArch64.

[abhinav92003]

LGTM. Okay to submit after resolving the remaining comment threads.

[abhinav92003]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches. https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133 @derekbruening @abhinav92003

Is this blocking this PR? If not, we can fix it separately; can you raise a new issue for it?

[shiptux]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches. https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133 @derekbruening @abhinav92003

Is this blocking this PR? If not, we can fix it separately; can you raise a new issue for it?

This makes dynamorio fail to compile on RISC-V. See the CI log in this PR.

[abhinav92003]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches. https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133 @derekbruening @abhinav92003

Is this blocking this PR? If not, we can fix it separately; can you raise a new issue for it?

This makes dynamorio fail to compile on RISC-V. See the CI log in this PR.

Oh I didn't notice that before since it's a warning asm_defines.asm:566: warning: "SYSNUM_REG" redefined that doesn't show up as a CI failure. The build failure seems to be due to a recent commit on this branch: Error: illegal operands 'ld s2 0(sp)'.

Regardless, I agree we should fix the warning. Like I said above, some of that #else part does seem targeted to only AArch64. Can you maybe add another conditional define there for now? And also move the risc-v definition to the same part of code? (We might want to refactor existing x86 code to also use a SYSNUM_REG constant to be consistent across all archs, but we can do it independently of this PR so as to not block it).

[shiptux]

Is SYSNUM_REG only used in aarch64? But it seems that it is defined in all arches. https://github.com/DynamoRIO/dynamorio/blob/master/core/arch/asm_defines.asm#L133 @derekbruening @abhinav92003

Is this blocking this PR? If not, we can fix it separately; can you raise a new issue for it?

This makes dynamorio fail to compile on RISC-V. See the CI log in this PR.

Oh I didn't notice that before since it's a warning asm_defines.asm:566: warning: "SYSNUM_REG" redefined that doesn't show up as a CI failure. The build failure seems to be due to a recent commit on this branch: Error: illegal operands 'ld s2 0(sp)'.

Regardless, I agree we should fix the warning. Like I said above, some of that #else part does seem targeted to only AArch64. Can you maybe add another conditional define there for now? And also move the risc-v definition to the same part of code? (We might want to refactor existing x86 code to also use a SYSNUM_REG constant to be consistent across all archs, but we can do it independently of this PR so as to not block it).

Ok. I fixed the CI error. It is not blocked by SYSNUM_REG.
I think move the define in another PR will be better.

[abhinav92003]

Ok. I fixed the CI error. It is not blocked by SYSNUM_REG. I think move the define in another PR will be better.

Okay. I raised #6027 for the cleanup.

[derekbruening]

Looks like @abhinav92003 already reviewed and approved: not sure why another review was requested, @shiptux ?

[shiptux]

Looks like @abhinav92003 already reviewed and approved: not sure why another review was requested, @shiptux ?

Sorry for requested that.
I saw that there have two reviewer, I mistaken for should both reviewer approval.

Sorry to bother you.

[shiptux]

@abhinav92003 @derekbruening It seems I don't have permission to merge this PR. (Need to merge by the project collaborators.) Could anyone help me?

[abhinav92003]

@abhinav92003 @derekbruening It seems I don't have permission to merge this PR. (Need to merge by the project collaborators.) Could anyone help me?

I sent you an invite for Committers access to the repo. Once you accept it, you should be able to submit.