[llvm] 84c033d - [LICM] [Coroutines] Don't hoist threadlocals within presplit coroutines

[Chuanqi Xu via llvm-commits]

Author: Chuanqi Xu
Date: 2023-06-07T10:25:47+08:00
New Revision: 84c033d9ba37ef51c0ca6e637ef954ed41d8c32d

URL: https://github.com/llvm/llvm-project/commit/84c033d9ba37ef51c0ca6e637ef954ed41d8c32d
DIFF: https://github.com/llvm/llvm-project/commit/84c033d9ba37ef51c0ca6e637ef954ed41d8c32d.diff

LOG: [LICM] [Coroutines] Don't hoist threadlocals within presplit coroutines

Close https://github.com/llvm/llvm-project/issues/63022

This is the following of https://reviews.llvm.org/D135550, which is
discussed in
https://discourse.llvm.org/t/rfc-unify-memory-effect-attributes/65579.
In my imagination, we could fix the issue fundamentally after we
introduces new memory kind thread id. But I am not very sure if we can
fix the issue fundamentally in time.

Besides that, I think the correctness is the most important. So it
should not be bad to land this given it is innocent.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D151774

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/LICM.cpp
    llvm/test/Transforms/LICM/sink-with-coroutine.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 9bfc0b49cb5c5..5079ef9920e98 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1226,6 +1226,15 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
 
     // Handle simple cases by querying alias analysis.
     MemoryEffects Behavior = AA->getMemoryEffects(CI);
+
+    // FIXME: we don't handle the semantics of thread local well. So that the
+    // address of thread locals are fake constants in coroutines. So We forbid
+    // to treat onlyReadsMemory call in coroutines as constants now. Note that
+    // it is possible to hide a thread local access in a onlyReadsMemory call.
+    // Remove this check after we handle the semantics of thread locals well.
+    if (Behavior.onlyReadsMemory() && CI->getFunction()->isPresplitCoroutine())
+      return false;
+
     if (Behavior.doesNotAccessMemory())
       return true;
     if (Behavior.onlyReadsMemory()) {

diff  --git a/llvm/test/Transforms/LICM/sink-with-coroutine.ll b/llvm/test/Transforms/LICM/sink-with-coroutine.ll
index f5ce25b70febc..0b3f2e1c630a1 100644
--- a/llvm/test/Transforms/LICM/sink-with-coroutine.ll
+++ b/llvm/test/Transforms/LICM/sink-with-coroutine.ll
@@ -59,20 +59,20 @@ define i64 @hoist_threadlocal() presplitcoroutine {
 ; CHECK-NEXT:    [[P:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:    br label [[LOOP_PREHEADER:%.*]]
 ; CHECK:       loop.preheader:
-; CHECK-NEXT:    [[THREAD_LOCAL_0:%.*]] = call ptr @llvm.threadlocal.address.p0(ptr @tls)
-; CHECK-NEXT:    [[THREAD_LOCAL_1:%.*]] = call ptr @llvm.threadlocal.address.p0(ptr @tls)
-; CHECK-NEXT:    [[CMP_0:%.*]] = icmp eq ptr [[THREAD_LOCAL_0]], [[THREAD_LOCAL_1]]
 ; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
 ; CHECK:       for.body:
 ; CHECK-NEXT:    [[I:%.*]] = phi i64 [ 0, [[LOOP_PREHEADER]] ], [ [[I_NEXT:%.*]], [[LOOP_END:%.*]] ]
 ; CHECK-NEXT:    [[I_NEXT]] = add i64 [[I]], 1
+; CHECK-NEXT:    [[THREAD_LOCAL_0:%.*]] = call ptr @llvm.threadlocal.address.p0(ptr @tls)
 ; CHECK-NEXT:    [[READONLY_0:%.*]] = call ptr @readonly_funcs()
 ; CHECK-NEXT:    [[SUSPEND:%.*]] = call i8 @llvm.coro.suspend(token none, i1 false)
 ; CHECK-NEXT:    switch i8 [[SUSPEND]], label [[EXIT:%.*]] [
 ; CHECK-NEXT:    i8 0, label [[AWAIT_READY:%.*]]
 ; CHECK-NEXT:    ]
 ; CHECK:       await.ready:
+; CHECK-NEXT:    [[THREAD_LOCAL_1:%.*]] = call ptr @llvm.threadlocal.address.p0(ptr @tls)
 ; CHECK-NEXT:    [[READONLY_1:%.*]] = call ptr @readonly_funcs()
+; CHECK-NEXT:    [[CMP_0:%.*]] = icmp eq ptr [[THREAD_LOCAL_0]], [[THREAD_LOCAL_1]]
 ; CHECK-NEXT:    [[CMP_1:%.*]] = icmp eq ptr [[READONLY_0]], [[READONLY_1]]
 ; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP_0]], [[CMP_1]]
 ; CHECK-NEXT:    br i1 [[CMP]], label [[NOT_REACHABLE:%.*]], label [[LOOP_END]]