Details
Diff Detail
Event Timeline
LGTM. Thanks!
llvm/test/CodeGen/RISCV/pr45303.ll | ||
---|---|---|
1 ↗ | (On Diff #252814) | Best to add a run line for rv32 too. |
Added rv32 run line.
@luismarques Thanks for the review.
I do not have commit access.
Please do not merge this until I have had the opportunity to ensure it works correctly with all threading models. I will approve it once I am satisfied this patch is correct.
Thanks for the patch. I added a couple of nits - please rename the test file to thread-pointer.ll to match what other backends do and make it more discoverable. Also, although there different is minor we do use update_llc_test_checks.py for essentially all RISC-V tests so it would be good to use that for thread-pointer.ll as well.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
841 | Nit: Should be DL to match the variable naming elsewhere. | |
llvm/test/CodeGen/RISCV/pr45303.ll | ||
1 ↗ | (On Diff #252825) | We normally put the smallest triple that make sense. In this case, just riscv32 and riscv64 are sufficient to exhibit the bheaviour. |
llvm/test/CodeGen/RISCV/pr45303.ll | ||
---|---|---|
6 ↗ | (On Diff #252825) | If you are going to use update_llc_test_checks.py then you should add nounwind, to avoid the CFI directives. |
llvm/test/CodeGen/RISCV/thread-pointer.ll | ||
---|---|---|
2 | Delete --check-prefix=CHECK |
After discussion with the RISC-V GCC Developers (including @kito-cheng), we have decided that both GCC and Clang will use __builtin_thread_pointer() to return the value in tp. I'm happy for this to land now.
Thanks @lenary for review.
I do not have commit access,
please commit this when you get time.
Nit: Should be DL to match the variable naming elsewhere.