[llvm] [LoopRotate][coroutines] Avoid hoisting addresses of thread-local variables outside loops in coroutines (PR #81937)

[Alan Zhao via llvm-commits]

https://github.com/alanzhao1 updated https://github.com/llvm/llvm-project/pull/81937

>From 300474861ee0124a317dec1a6ba5700d18be9cb7 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Thu, 15 Feb 2024 14:43:42 -0800
Subject: [PATCH] [LoopRotate][coroutines] Avoid hoisting addresses of
 thread-local variables outside loops in coroutines

Because loops in coroutines may have a co_await statement that
reschedules the coroutine to another thread, we cannot cache addresses
of thread-local variables obtained inside a loop by moving the
computation of thoes addresses outside a loop.

Since LLVM doesn't have a model for coroutine memory accesses, this
patch fixes this bug by disabling this optimization for coroutines in
the same way as https://reviews.llvm.org/D135550 and
https://reviews.llvm.org/D151774.
---
 llvm/lib/Transforms/Utils/LoopRotationUtils.cpp | 11 ++++++++++-
 llvm/test/Transforms/LoopRotate/coroutine.ll    |  5 +++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
index b12c1b9bb9d5e4..adae796ee02997 100644
--- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -612,7 +612,16 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
       // memory (without proving that the loop doesn't write).
       if (L->hasLoopInvariantOperands(Inst) && !Inst->mayReadFromMemory() &&
           !Inst->mayWriteToMemory() && !Inst->isTerminator() &&
-          !isa<DbgInfoIntrinsic>(Inst) && !isa<AllocaInst>(Inst)) {
+          !isa<DbgInfoIntrinsic>(Inst) && !isa<AllocaInst>(Inst) &&
+          // It is not safe to hoist the value of these instructions in
+          // coroutines, as the addresses of otherwise eligible variables (e.g.
+          // thread-local variables and errno) may change if the coroutine is
+          // resumed in a different thread.Therefore, we disable this
+          // optimization for correctness. However, this may block other correct
+          // optimizations.
+          // FIXME: This should be reverted once we have a better model for
+          // memory access in coroutines.
+          !Inst->getFunction()->isPresplitCoroutine()) {
 
         if (LoopEntryBranch->getParent()->IsNewDbgInfoFormat &&
             !NextDbgInsts.empty()) {
diff --git a/llvm/test/Transforms/LoopRotate/coroutine.ll b/llvm/test/Transforms/LoopRotate/coroutine.ll
index 9d7bf595e21cdc..4544ac5e72bbe5 100644
--- a/llvm/test/Transforms/LoopRotate/coroutine.ll
+++ b/llvm/test/Transforms/LoopRotate/coroutine.ll
@@ -21,8 +21,9 @@ define void @foo() #0 {
 ; CHECK-NEXT:    ret void
 ; CHECK:       cond.end:
 ; CHECK-NEXT:    call void @bar()
-; CHECK-NEXT:    [[TMP2:%.*]] = load i32, ptr [[TMP0]], align 4
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT:    [[TMP2:%.*]] = tail call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @threadlocalint)
+; CHECK-NEXT:    [[TMP3:%.*]] = load i32, ptr [[TMP2]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[TMP3]], 0
 ; CHECK-NEXT:    br i1 [[CMP]], label [[COND_END]], label [[WHILE_COND_COND_FALSE_CRIT_EDGE:%.*]]
 ;
 entry: