[WIP] gh-112535: Implement fallback implementation of _Py_ThreadId()#113094
Uh oh!
There was an error while loading. Please reload this page.
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
corona10force-pushed the gh-112535-fallback branch from d58ad35 to 5977313Compare
corona10force-pushed the gh-112535-fallback branch from 5977313 to 7015d7fCompare
corona10 commented Dec 14, 2023
Include/object.h Outdated
| // 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) |
MemberAuthor
There was a problem hiding this comment.
@colesbury Please let me know, it would be proper explaination.
MemberAuthor
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 |
MemberAuthor
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..
Contributor
colesbury left a comment
colesbury left a comment 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
| tid = (uintptr_t)__builtin_thread_pointer(); | ||
| #elif defined(thread_local) || defined(__GNUC__) | ||
| #if defined(thread_local) | ||
| static thread_local int __tp = 0; |
Contributor
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.
Contributor
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.
corona10 changed the title 
…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 <[email protected]>
…le.c (pythonGH-113173) Fix compiler waarnings in Modules/_xxinterpqueuesmodule.c
corona10force-pushed the gh-112535-fallback branch from 2c2dcdb to e3e3973Comparecorona10 requested review from a team, AlexWaygood, abalkin, ericsnowcurrently, erlend-aasland, ethanfurman, gvanrossum, markshannon, pganssle and rhettinger as code ownerscorona10 removed request for a team, AlexWaygood, abalkin, ericsnowcurrently, erlend-aasland, ethanfurman, gvanrossum, markshannon, pganssle, rhettinger and vstinnerSign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
18 participants
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