]> granicus.if.org Git - llvm/commitdiff
[CodeGen] Be as conservative about atomic accesses as for volatile
authorPhilip Reames <listmail@philipreames.com>
Fri, 1 Feb 2019 22:58:52 +0000 (22:58 +0000)
committerPhilip Reames <listmail@philipreames.com>
Fri, 1 Feb 2019 22:58:52 +0000 (22:58 +0000)
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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@352937 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/MachineInstr.cpp
lib/CodeGen/MachinePipeliner.cpp
lib/CodeGen/ScheduleDAGInstrs.cpp

index 7ab17fce04d3559d767e1e6ed042895e01e42edd..ca43804481bfa1e19b86a86573f7091d070bf666 100644 (file)
@@ -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;
index fab44512914c1536cdf9d35b7d8bc6a2ca1f38a9..1e729d4851aba651d345efa1693abd0c0a8db7b7 100644 (file)
@@ -2766,7 +2766,9 @@ void SwingSchedulerDAG::updateMemOperands(MachineInstr &NewMI,
     return;
   SmallVector<MachineMemOperand *, 2> 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;
index ab503206a51b0363986b389709c5d4f0767d5685..3e68e09c44e3b499f77f841242ede33369c4a160 100644 (file)
@@ -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()) {