-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[OpenMP][RISCV] Add riscv32 support #99494
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
base: main
Are you sure you want to change the base?
Conversation
Tagging my colleague @mikhailramalho who's been doing some RV32 stuff recently so can at least provide additional testing if not a full code review. |
@wangpc-pp could you provide instructions on how to test this patch? I can use the setup I have to test libc to test your patch. |
Thanks!
You may set |
You may also need changes in OMPT and OMPD. They are arch dependent I think. |
I just searched "riscv64" and made riscv32 changes around riscv64 codes. We may not need to change OMPT/OMPD, or they are lack of riscv64 support? |
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.
The structure of the code change looks good to me, but I'm not familiar with RISC-V arch so I'm not sure the technical soundness of the asm changes. I suppose it has been tested.
Reverse ping. |
CMake flags and some core changes are added to support riscv32 platform basically. Fix llvm#99427
65510fd
to
ce91cf3
Compare
I can't guarantee the correctness of this implementation... It's hard to test. |
We need to test it to ensure that most of it works as expected. |
For ompt, only the tests use some arch/isa specific assumptions. If the tests succeed, no changes are necessary. Otherwise, some tweaks might be necessary in runtime/test/ompt/callback.h look for ARCH specific code. Ompd is usually only activated for selected supported architectures. If you want to support OpenMP debugging for the architecture, you would need to check whether it works out of the box, or whether changes are necessary. |
Can you run the suite https://github.com/OpenMP-Validation-and-Verification/OpenMP_VV? Actually there are several tests failed for riscv64. I think what we need to do now is to improve the riscv64 implementation first and then riscv32. |
CMake flags and some core changes are added to support riscv32
platform basically.
Fix #99427