[PATCH] D57596: [CodeGen] Be conservative about atomic accesses as for volatile

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 184850.
reames added a comment.

Update w/requested todos and a simplification.  It occurred to me that LoadSDNode is not a subclass of AtomicSDNode, so anything of static type LoadSDNode doesn't need the conservative atomicity check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57596/new/

https://reviews.llvm.org/D57596

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


Index: lib/CodeGen/ScheduleDAGInstrs.cpp
===================================================================
--- lib/CodeGen/ScheduleDAGInstrs.cpp
+++ lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -131,7 +131,8 @@
                                          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()) {
Index: lib/CodeGen/MachinePipeliner.cpp
===================================================================
--- lib/CodeGen/MachinePipeliner.cpp
+++ lib/CodeGen/MachinePipeliner.cpp
@@ -2766,7 +2766,9 @@
     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: lib/CodeGen/MachineInstr.cpp
===================================================================
--- lib/CodeGen/MachineInstr.cpp
+++ lib/CodeGen/MachineInstr.cpp
@@ -1305,6 +1305,8 @@
 
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57596.184850.patch
Type: text/x-patch
Size: 1795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190201/00a6f617/attachment.bin>