From 00056ed0e66d58ee5fc57f07e6a7a9976e0731ab Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 1 Feb 2019 22:58:52 +0000 Subject: [PATCH] [CodeGen] Be as conservative about atomic accesses as for volatile Background: At the moment, we record the AtomicOrdering of an access in the MMO, but also mark any atomic access as volatile in SelectionDAG. I'm working towards separating that. See https://reviews.llvm.org/D57601 for context. Update all usages of isVolatile in lib/CodeGen to preserve behaviour once atomic MMOs stop being also volatile. This is NFC in it's current form, but is essential for correctness once we make that final change. It useful to keep in mind that AtomicSDNode is not a parent of LoadSDNode, StoreSDNode, or LSBaseSDNode. As a result, any call to isVolatile on one of those static types doesn't need a companion isAtomic check. We should probably adjust that class hierarchy long term, but for now, that seperation is useful. I'm deliberately being conservative about handling. I want the change to stop adding volatile to be NFC itself, and then will work through places where we can be less conservative for atomics one by one in separate changes w/tests. Differential Revision: https://reviews.llvm.org/D57596 llvm-svn: 352937 --- llvm/lib/CodeGen/MachineInstr.cpp | 2 ++ llvm/lib/CodeGen/MachinePipeliner.cpp | 4 +++- llvm/lib/CodeGen/ScheduleDAGInstrs.cpp | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp index 7ab17fc..ca43804 100644 --- a/llvm/lib/CodeGen/MachineInstr.cpp +++ b/llvm/lib/CodeGen/MachineInstr.cpp @@ -1305,6 +1305,8 @@ bool MachineInstr::isDereferenceableInvariantLoad(AliasAnalysis *AA) const { for (MachineMemOperand *MMO : memoperands()) { if (MMO->isVolatile()) return false; + // TODO: Figure out whether isAtomic is really necessary (see D57601). + if (MMO->isAtomic()) return false; if (MMO->isStore()) return false; if (MMO->isInvariant() && MMO->isDereferenceable()) continue; diff --git a/llvm/lib/CodeGen/MachinePipeliner.cpp b/llvm/lib/CodeGen/MachinePipeliner.cpp index fab4451..1e729d4 100644 --- a/llvm/lib/CodeGen/MachinePipeliner.cpp +++ b/llvm/lib/CodeGen/MachinePipeliner.cpp @@ -2766,7 +2766,9 @@ void SwingSchedulerDAG::updateMemOperands(MachineInstr &NewMI, return; SmallVector NewMMOs; for (MachineMemOperand *MMO : NewMI.memoperands()) { - if (MMO->isVolatile() || (MMO->isInvariant() && MMO->isDereferenceable()) || + // TODO: Figure out whether isAtomic is really necessary (see D57601). + if (MMO->isVolatile() || MMO->isAtomic() || + (MMO->isInvariant() && MMO->isDereferenceable()) || (!MMO->getValue())) { NewMMOs.push_back(MMO); continue; diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp index ab50320..3e68e09 100644 --- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -131,7 +131,8 @@ static bool getUnderlyingObjectsForInstr(const MachineInstr *MI, const DataLayout &DL) { auto allMMOsOkay = [&]() { for (const MachineMemOperand *MMO : MI->memoperands()) { - if (MMO->isVolatile()) + // TODO: Figure out whether isAtomic is really necessary (see D57601). + if (MMO->isVolatile() || MMO->isAtomic()) return false; if (const PseudoSourceValue *PSV = MMO->getPseudoValue()) { -- 2.7.4