[llvm] 36e73e4 - [LICM][NFCish] Consider all calls in a presplit coroutine unsinkable/unhoistable (#81951)

[via llvm-commits]

Author: Arthur Eubanks
Date: 2024-02-16T10:08:00-08:00
New Revision: 36e73e4edcf31855bf5219a750ce8e6c76a91524

URL: https://github.com/llvm/llvm-project/commit/36e73e4edcf31855bf5219a750ce8e6c76a91524
DIFF: https://github.com/llvm/llvm-project/commit/36e73e4edcf31855bf5219a750ce8e6c76a91524.diff

LOG: [LICM][NFCish] Consider all calls in a presplit coroutine unsinkable/unhoistable (#81951)

NFCish since previously we'd return false for all presplit coroutines
anyway. This clarifies things a bit.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/LICM.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 9ec9c31e48e0b6..546e718cb50851 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1217,6 +1217,14 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
     if (CI->isConvergent())
       return false;
 
+    // FIXME: Current LLVM IR semantics don't work well with coroutines and
+    // thread local globals. We currently treat getting the address of a thread
+    // local global as not accessing memory, even though it may not be a
+    // constant throughout a function with coroutines. Remove this check after
+    // we better model semantics of thread local globals.
+    if (CI->getFunction()->isPresplitCoroutine())
+      return false;
+
     using namespace PatternMatch;
     if (match(CI, m_Intrinsic<Intrinsic::assume>()))
       // Assumes don't actually alias anything or throw
@@ -1225,14 +1233,6 @@ 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()) {