[PATCH] D92662: [Clang][Coroutine] Drop const attribute on pthread_self when coroutine is enabled

[Xun Li via Phabricator via cfe-commits]

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()) &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92662.309548.patch
Type: text/x-patch
Size: 2811 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201204/489f6ac7/attachment.bin>