Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2024
Merged

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions llvm/lib/Transforms/Scalar/LICM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()) {
Expand Down