Page MenuHomePhabricator

[RISC-V] Support __builtin_thread_pointer
ClosedPublic

Authored by kamleshbhalui on Mar 26 2020, 12:13 AM.

Diff Detail

Event Timeline

kamleshbhalui created this revision.Mar 26 2020, 12:13 AM
luismarques requested changes to this revision.Mar 26 2020, 4:36 AM

Please add tests.

This revision now requires changes to proceed.Mar 26 2020, 4:36 AM

nit and Added the tests.

luismarques accepted this revision.Mar 26 2020, 6:10 AM

LGTM. Thanks!

llvm/test/CodeGen/RISCV/pr45303.ll
1 ↗(On Diff #252814)

Best to add a run line for rv32 too.

This revision is now accepted and ready to land.Mar 26 2020, 6:10 AM

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.

asb added a comment.Mar 26 2020, 6:34 AM

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.

luismarques added inline comments.Mar 26 2020, 6:37 AM
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.

nits and updated test.

MaskRay accepted this revision.Mar 26 2020, 9:02 AM
MaskRay added inline comments.
llvm/test/CodeGen/RISCV/thread-pointer.ll
3

Delete --check-prefix=CHECK

lenary accepted this revision.Mar 27 2020, 7:05 AM

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.

This revision was automatically updated to reflect the committed changes.