Skip to content

Commit 00056ed

Browse files
committed
[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
1 parent b392ac9 commit 00056ed

File tree

3 files changed

+7
-2
lines changed

3 files changed

+7
-2
lines changed

llvm/lib/CodeGen/MachineInstr.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,6 +1305,8 @@ bool MachineInstr::isDereferenceableInvariantLoad(AliasAnalysis *AA) const {
13051305

13061306
for (MachineMemOperand *MMO : memoperands()) {
13071307
if (MMO->isVolatile()) return false;
1308+
// TODO: Figure out whether isAtomic is really necessary (see D57601).
1309+
if (MMO->isAtomic()) return false;
13081310
if (MMO->isStore()) return false;
13091311
if (MMO->isInvariant() && MMO->isDereferenceable())
13101312
continue;

llvm/lib/CodeGen/MachinePipeliner.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2766,7 +2766,9 @@ void SwingSchedulerDAG::updateMemOperands(MachineInstr &NewMI,
27662766
return;
27672767
SmallVector<MachineMemOperand *, 2> NewMMOs;
27682768
for (MachineMemOperand *MMO : NewMI.memoperands()) {
2769-
if (MMO->isVolatile() || (MMO->isInvariant() && MMO->isDereferenceable()) ||
2769+
// TODO: Figure out whether isAtomic is really necessary (see D57601).
2770+
if (MMO->isVolatile() || MMO->isAtomic() ||
2771+
(MMO->isInvariant() && MMO->isDereferenceable()) ||
27702772
(!MMO->getValue())) {
27712773
NewMMOs.push_back(MMO);
27722774
continue;

llvm/lib/CodeGen/ScheduleDAGInstrs.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ static bool getUnderlyingObjectsForInstr(const MachineInstr *MI,
131131
const DataLayout &DL) {
132132
auto allMMOsOkay = [&]() {
133133
for (const MachineMemOperand *MMO : MI->memoperands()) {
134-
if (MMO->isVolatile())
134+
// TODO: Figure out whether isAtomic is really necessary (see D57601).
135+
if (MMO->isVolatile() || MMO->isAtomic())
135136
return false;
136137

137138
if (const PseudoSourceValue *PSV = MMO->getPseudoValue()) {

0 commit comments

Comments
 (0)