Skip to content

gh-112535: Update _Py_ThreadId() to support RISC-V #113084

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 14, 2023
Merged

gh-112535: Update _Py_ThreadId() to support RISC-V #113084

merged 1 commit into from
Dec 14, 2023
+7 −0

Conversation

furkanonder
Copy link
Contributor

@furkanonder furkanonder commented Dec 14, 2023 •
edited
Loading

  • Clang supports __builtin_thread_pointer for RISC-V from clang 11.0.0
  • GCC supports __builtin_thread_pointer for RISC-V from GCC 10.2.0 (The Clang team said that checking the __builtin_thread_pointer with __has_builtin but not safe for GCC)

@bedevere-app bedevere-app bot mentioned this pull request Dec 14, 2023
Closed
@furkanonder
Copy link
Contributor Author

furkanonder commented Dec 14, 2023 •
edited
Loading

GCC

 ./configure --disable-gil --with-pydebug

Test:

user@starfive:~/cpython$ ./python -m test -j3

Output:

== Tests result: FAILURE ==

12 tests skipped:
    test.test_asyncio.test_windows_events
    test.test_asyncio.test_windows_utils test_devpoll test_ioctl
    test_kqueue test_launcher test_msvcrt test_perf_profiler
    test_startfile test_winconsoleio test_winreg test_wmi

10 tests skipped (resource denied):
    test_curses test_peg_generator test_smtpnet test_socketserver
    test_tkinter test_ttk test_urllib2net test_urllibnet test_winsound
    test_zipfile64

1 test failed:
    test_cmd_line

446 tests OK.

Total duration: 35 min 12 sec
Total tests: run=40,908 failures=2 skipped=1,401
Total test files: run=459/469 failed=1 skipped=12 resource_denied=10
Result: FAILURE

The failed test_cmd_line test was already an existing issue for RISC-V.

@furkanonder
Copy link
Contributor Author

furkanonder commented Dec 14, 2023 •
edited
Loading

CLANG

./configure --disable-gil --with-pydebug --with-cxx-main=/usr/bin/clang++ CC=/usr/bin/clang

Test:

user@starfive:~/cpython$ ./python -m test -j3

Output:

== Tests result: FAILURE ==

12 tests skipped:
    test.test_asyncio.test_windows_events
    test.test_asyncio.test_windows_utils test_devpoll test_ioctl
    test_kqueue test_launcher test_msvcrt test_perf_profiler
    test_startfile test_winconsoleio test_winreg test_wmi

10 tests skipped (resource denied):
    test_curses test_peg_generator test_smtpnet test_socketserver
    test_tkinter test_ttk test_urllib2net test_urllibnet test_winsound
    test_zipfile64

3 tests failed:
    test.test_gdb.test_backtrace test.test_gdb.test_pretty_print
    test_cmd_line

444 tests OK.

Total duration: 31 min 45 sec
Total tests: run=40,908 failures=24 skipped=1,415
Total test files: run=459/469 failed=3 skipped=12 resource_denied=10
Result: FAILURE

The failed test_cmd_line test and test.test_gdb.test_backtrace, test.test_gdb.test_pretty_print were already existing issues for RISC-V.

corona10
corona10 reviewed Dec 14, 2023
View reviewed changes
Include/object.h
tid = (uintptr_t)__builtin_thread_pointer();
#else
// tp is Thread Pointer provided by the RISC-V ABI.
__asm__ ("mv %0, tp" : "=r" (tid));
Copy link
Member

@corona10 corona10 Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

https://gcc.godbolt.org/z/TYe716Wa7

I cross-checked the assembly code from Godbolt, too.
(Both 32bit and 64bit)

@corona10 corona10 requested a review from colesbury December 14, 2023 03:27
corona10
corona10 approved these changes Dec 14, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

LGTM but waiting @colesbury too.

@corona10 corona10 merged commit f34e22c into python:main Dec 14, 2023
corona10 pushed a commit to corona10/cpython that referenced this pull request Dec 15, 2023
@furkanonder @corona10
pythongh-112535: Update _Py_ThreadId() to support RISC-V (pythongh-11…
eb9bc91 
…3084)

Update _Py_ThreadId() to support RISC-V
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@furkanonder @aisk
pythongh-112535: Update _Py_ThreadId() to support RISC-V (pythongh-11…
8588abe 
…3084)

Update _Py_ThreadId() to support RISC-V
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@furkanonder @Glyphack
pythongh-112535: Update _Py_ThreadId() to support RISC-V (pythongh-11…
f33995b 
…3084)

Update _Py_ThreadId() to support RISC-V
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colesbury colesbury colesbury approved these changes

@corona10 corona10 corona10 approved these changes

Assignees
No one assigned
Labels
skip news topic-free-threading
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@furkanonder @colesbury @corona10