lxfind created this revision. Herald added subscribers: hoy, modimo, wenlei, modocache. lxfind requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
This patch is to address https://bugs.llvm.org/show_bug.cgi?id=47833 A relevant discussion can also be found in http://lists.llvm.org/pipermail/llvm-dev/2020-November/146766.html pthread_self() from glibc is defined with "__attribute__ ((__const__))". The const attribute tells the compiler that it does not read nor write any global state and hence always return the same result. Hence in the following code: auto x1 = pthread_self(); ... auto x2 = pthread_self(); the second call to pthread_self() can be optimized out. This has been correct until coroutines. With coroutines, we can have code like this: auto x1 = pthread_self(); co_await ... auto x2 = pthread_self(); Now because of the co_await, the function can suspend and resume in a different thread, in which case the second call to pthread_self() should return a different result than the first one. Unfortunately LLVM will still optimize out the second call in the case of coroutines. To fix the issue, this patch drops the readnone attribute from the pthread_self function in Clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D92662 Files: clang/lib/Sema/SemaDecl.cpp clang/test/CodeGenCoroutines/coro-pthread_self.cpp Index: clang/test/CodeGenCoroutines/coro-pthread_self.cpp =================================================================== --- /dev/null +++ clang/test/CodeGenCoroutines/coro-pthread_self.cpp @@ -0,0 +1,58 @@ +// RUN: %clang -fcoroutines-ts -std=c++14 -O3 -emit-llvm -S %s -o - | FileCheck %s + +#include "Inputs/coroutine.h" + +namespace coro = std::experimental::coroutines_v1; + +typedef void *pthread_t; +pthread_t pthread_self(void) __attribute__((__const__)); + +struct awaitable { + bool await_ready() { return false; } + void await_suspend(coro::coroutine_handle<> h); + void await_resume() {} +}; +awaitable switch_to_new_thread(); + +struct task { + struct promise_type { + task get_return_object() { return {}; } + coro::suspend_never initial_suspend() { return {}; } + coro::suspend_never final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} + }; +}; + +void check(pthread_t p1, pthread_t p2); + +task resuming_on_new_thread() { + auto pthread1 = pthread_self(); + co_await switch_to_new_thread(); + auto pthread2 = pthread_self(); + check(pthread1, pthread2); +} + +void non_coroutine() { + auto pthread1 = pthread_self(); + check(pthread1, pthread1); + auto pthread2 = pthread_self(); + check(pthread1, pthread2); +} + +// CHECK-LABEL: define void @_Z13non_coroutinev() +// CHECK-NEXT: entry: +// CHECK-NEXT: %call = tail call i8* @_Z12pthread_selfv() +// CHECK-NEXT: tail call void @_Z5checkPvS_(i8* %call, i8* %call) +// CHECK-NEXT: %call1 = tail call i8* @_Z12pthread_selfv() +// CHECK-NEXT: tail call void @_Z5checkPvS_(i8* %call, i8* %call1) +// CHECK-NEXT: ret void +// CHECK-NEXT: } + +// CHECK-LABEL: define internal fastcc void @_Z22resuming_on_new_threadv.resume +// CHECK: %[[CALL:.+]] = invoke i8* @_Z12pthread_selfv() +// CHECK-NEXT: to label %[[CONT:.+]] unwind label %{{.+}} +// CHECK: [[CONT]]: +// CHECK-NEXT: %[[RELOAD_ADDR:.+]] = getelementptr inbounds %_Z22resuming_on_new_threadv.Frame, %_Z22resuming_on_new_threadv.Frame* %FramePtr, i64 0, i32 {{.+}} +// CHECK-NEXT: %[[RELOAD:.+]] = load i8*, i8** %[[RELOAD_ADDR]], align 8 +// CHECK-NEXT: invoke void @_Z5checkPvS_(i8* %[[RELOAD]], i8* %[[CALL]]) Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -14938,6 +14938,12 @@ IdentifierInfo *Name = FD->getIdentifier(); if (!Name) return; + + if (getLangOpts().Coroutines && Name->isStr("pthread_self") && + FD->hasAttr<ConstAttr>()) { + FD->dropAttr<ConstAttr>(); + } + if ((!getLangOpts().CPlusPlus && FD->getDeclContext()->isTranslationUnit()) || (isa<LinkageSpecDecl>(FD->getDeclContext()) &&
Index: clang/test/CodeGenCoroutines/coro-pthread_self.cpp =================================================================== --- /dev/null +++ clang/test/CodeGenCoroutines/coro-pthread_self.cpp @@ -0,0 +1,58 @@ +// RUN: %clang -fcoroutines-ts -std=c++14 -O3 -emit-llvm -S %s -o - | FileCheck %s + +#include "Inputs/coroutine.h" + +namespace coro = std::experimental::coroutines_v1; + +typedef void *pthread_t; +pthread_t pthread_self(void) __attribute__((__const__)); + +struct awaitable { + bool await_ready() { return false; } + void await_suspend(coro::coroutine_handle<> h); + void await_resume() {} +}; +awaitable switch_to_new_thread(); + +struct task { + struct promise_type { + task get_return_object() { return {}; } + coro::suspend_never initial_suspend() { return {}; } + coro::suspend_never final_suspend() noexcept { return {}; } + void return_void() {} + void unhandled_exception() {} + }; +}; + +void check(pthread_t p1, pthread_t p2); + +task resuming_on_new_thread() { + auto pthread1 = pthread_self(); + co_await switch_to_new_thread(); + auto pthread2 = pthread_self(); + check(pthread1, pthread2); +} + +void non_coroutine() { + auto pthread1 = pthread_self(); + check(pthread1, pthread1); + auto pthread2 = pthread_self(); + check(pthread1, pthread2); +} + +// CHECK-LABEL: define void @_Z13non_coroutinev() +// CHECK-NEXT: entry: +// CHECK-NEXT: %call = tail call i8* @_Z12pthread_selfv() +// CHECK-NEXT: tail call void @_Z5checkPvS_(i8* %call, i8* %call) +// CHECK-NEXT: %call1 = tail call i8* @_Z12pthread_selfv() +// CHECK-NEXT: tail call void @_Z5checkPvS_(i8* %call, i8* %call1) +// CHECK-NEXT: ret void +// CHECK-NEXT: } + +// CHECK-LABEL: define internal fastcc void @_Z22resuming_on_new_threadv.resume +// CHECK: %[[CALL:.+]] = invoke i8* @_Z12pthread_selfv() +// CHECK-NEXT: to label %[[CONT:.+]] unwind label %{{.+}} +// CHECK: [[CONT]]: +// CHECK-NEXT: %[[RELOAD_ADDR:.+]] = getelementptr inbounds %_Z22resuming_on_new_threadv.Frame, %_Z22resuming_on_new_threadv.Frame* %FramePtr, i64 0, i32 {{.+}} +// CHECK-NEXT: %[[RELOAD:.+]] = load i8*, i8** %[[RELOAD_ADDR]], align 8 +// CHECK-NEXT: invoke void @_Z5checkPvS_(i8* %[[RELOAD]], i8* %[[CALL]]) Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -14938,6 +14938,12 @@ IdentifierInfo *Name = FD->getIdentifier(); if (!Name) return; + + if (getLangOpts().Coroutines && Name->isStr("pthread_self") && + FD->hasAttr<ConstAttr>()) { + FD->dropAttr<ConstAttr>(); + } + if ((!getLangOpts().CPlusPlus && FD->getDeclContext()->isTranslationUnit()) || (isa<LinkageSpecDecl>(FD->getDeclContext()) &&
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits