[WIP] gh-112535: Implement fallback implementation of _Py_ThreadId() #113094
+3,133
−2,371
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
d58ad35 to
5977313
Compare
5977313 to
7015d7f
Compare
corona10
commented
Dec 14, 2023
Include/object.h
Outdated
| @@ -283,6 +293,13 @@ _Py_ThreadId(void) | |||
| // Both GCC and Clang have supported __builtin_thread_pointer | |||
| // for s390 from long time ago. | |||
| tid = (uintptr_t)__builtin_thread_pointer(); | |||
| #elif defined(HAVE_THREAD_ID_FALLBACK) | |||
There was a problem hiding this comment.
@colesbury Please let me know, it would be proper explaination.
|
FYI, I checked that it works properly with removing platform-specific implementation in my local. diff --git a/Include/object.h b/Include/object.h
index 7c7feff085..5efeda2a00 100644
--- a/Include/object.h
+++ b/Include/object.h
@@ -244,47 +244,7 @@ static inline uintptr_t
_Py_ThreadId(void)
{
uintptr_t tid;
-#if defined(_MSC_VER) && defined(_M_X64)
- tid = __readgsqword(48);
-#elif defined(_MSC_VER) && defined(_M_IX86)
- tid = __readfsdword(24);
-#elif defined(_MSC_VER) && defined(_M_ARM64)
- tid = __getReg(18);
-#elif defined(__i386__)
- __asm__("movl %%gs:0, %0" : "=r" (tid)); // 32-bit always uses GS
-#elif defined(__MACH__) && defined(__x86_64__)
- __asm__("movq %%gs:0, %0" : "=r" (tid)); // x86_64 macOSX uses GS
-#elif defined(__x86_64__)
- __asm__("movq %%fs:0, %0" : "=r" (tid)); // x86_64 Linux, BSD uses FS
-#elif defined(__arm__)
- __asm__ ("mrc p15, 0, %0, c13, c0, 3\nbic %0, %0, #3" : "=r" (tid));
-#elif defined(__aarch64__) && defined(__APPLE__)
- __asm__ ("mrs %0, tpidrro_el0" : "=r" (tid));
-#elif defined(__aarch64__)
- __asm__ ("mrs %0, tpidr_el0" : "=r" (tid));
-#elif defined(__powerpc64__)
- #if defined(__clang__) && _Py__has_builtin(__builtin_thread_pointer)
- tid = (uintptr_t)__builtin_thread_pointer();
- #else
- // r13 is reserved for use as system thread ID by the Power 64-bit ABI.
- register uintptr_t tp __asm__ ("r13");
- __asm__("" : "=r" (tp));
- tid = tp;
- #endif
-#elif defined(__powerpc__)
- #if defined(__clang__) && _Py__has_builtin(__builtin_thread_pointer)
- tid = (uintptr_t)__builtin_thread_pointer();
- #else
- // r2 is reserved for use as system thread ID by the Power 32-bit ABI.
- register uintptr_t tp __asm__ ("r2");
- __asm__ ("" : "=r" (tp));
- tid = tp;
- #endif
-#elif defined(__s390__) && defined(__GNUC__)
- // Both GCC and Clang have supported __builtin_thread_pointer
- // for s390 from long time ago.
- tid = (uintptr_t)__builtin_thread_pointer();
-#elif defined(thread_local) || defined(__GNUC__)
+#if defined(thread_local) || defined(__GNUC__)
# if defined(thread_local)
static thread_local int __tp = 0;
#elif defined(__GNUC__)
|
corona10
commented
Dec 14, 2023
| #endif | ||
| // gh-112535: Using characteristics of TLS address mapping. | ||
| // The address of the thread-local variable is not equal with the actual thread pointer, | ||
| // However, it has the property of being determined by loader at runtime, with no duplication of values |
There was a problem hiding this comment.
I am not sure it's proper explaination. Some elements are initialzied at the startup and some elements are blahblahblah..
colesbury
reviewed
Dec 14, 2023
There was a problem hiding this comment.
We want the thread-local variable to be in a C file:
_PyThread_Id()should fall back to a function call that returns the thread id- You can put that function in
pystate.cand return the address of_Py_tss_tstate. (We can't return the value of_Py_tss_tstatebecause that might be NULL)
Include/object.h
Outdated
| @@ -283,6 +283,19 @@ _Py_ThreadId(void) | |||
| // Both GCC and Clang have supported __builtin_thread_pointer | |||
| // for s390 from long time ago. | |||
| tid = (uintptr_t)__builtin_thread_pointer(); | |||
| #elif defined(thread_local) || defined(__GNUC__) | |||
| #if defined(thread_local) | |||
| static thread_local int __tp = 0; | |||
There was a problem hiding this comment.
I don't think this will do what we want.__tp will be a unique variable in each C file that includes this header.
There was a problem hiding this comment.
To expand on this: if you declare a static variable inside a static inline function, then that variable is different in each translation unit (C file) that includes the static inline function.
…13087) Also make test_copymode_symlink_to_symlink in test_shutil more strict.
…H-113071) When new mailbox.Maildir methods were added for 3.13.0a2, their documentation was added at the end of the mailbox.Maildir section instead of grouping them with other methods Maildir adds to Mailbox. This commit moves the new methods' documentation adjacent to documentation for existing Maildir-specific methods, so that the "special remarks" for common methods remains at the end.
…honGH-112770) It was raised in two cases: * in the import statement when looking up __import__ * in pickling some builtin type when looking up built-ins iter, getattr, etc.
…ference/simple_stmts.rst` (python#113107)
…ython#113128) On Windows, Process.terminate() no longer sets the returncode attribute to always call WaitForSingleObject() in Process.wait(). Previously, sometimes the process was still running after TerminateProcess() even if GetExitCodeProcess() is not STILL_ACTIVE.
….parseaddr() (python#111116) Detect email address parsing errors and return empty tuple to indicate the parsing error (old API). Add an optional 'strict' parameter to getaddresses() and parseaddr() functions. Patch by Thomas Dwyer. Co-Authored-By: Thomas Dwyer <github@tomd.tel>
…le.c (pythonGH-113173) Fix compiler waarnings in Modules/_xxinterpqueuesmodule.c
2c2dcdb to
e3e3973
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_Py_ThreadId()work on PowerPC, IBM Z, etc. #112535