[PATCH] D57802: Be conservative about unordered accesses for the moment

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: jlebar, stoklund, kzhuravl.
Herald added subscribers: jfb, bollu, mcrosier.

Background: As described in https://reviews.llvm.org/D57601, I'm working towards separating volatile and atomic in the MMO uses for atomic instructions.

In https://reviews.llvm.org/D57593, I fixed a bug where isUnordered was returning the wrong result, but didn't account for the fact I was getting slightly ahead of myself.  While both uses of isUnordered are correct (as far as I can tell), we don't have tests to demonstrate this and being aggressive gets in the way of having the removal of volatile truly be non-functional.  Once D57601 <https://reviews.llvm.org/D57601> lands, I will return to these call sites, revert this patch, and add the appropriate tests to show the expected behaviour.


https://reviews.llvm.org/D57802

Files:
  lib/CodeGen/ImplicitNullChecks.cpp
  lib/CodeGen/MachineInstr.cpp


Index: lib/CodeGen/MachineInstr.cpp
===================================================================
--- lib/CodeGen/MachineInstr.cpp
+++ lib/CodeGen/MachineInstr.cpp
@@ -1284,8 +1284,10 @@
     return true;
 
   // Check if any of our memory operands are ordered.
+  // TODO: This should probably be be isUnordered (see D57601), but the callers
+  // need audited and test cases written to be sure.
   return llvm::any_of(memoperands(), [](const MachineMemOperand *MMO) {
-    return !MMO->isUnordered();
+    return MMO->isVolatile() || MMO->isAtomic();
   });
 }
 
Index: lib/CodeGen/ImplicitNullChecks.cpp
===================================================================
--- lib/CodeGen/ImplicitNullChecks.cpp
+++ lib/CodeGen/ImplicitNullChecks.cpp
@@ -235,8 +235,11 @@
   assert(!llvm::any_of(MI->operands(), IsRegMask) &&
          "Calls were filtered out above!");
 
-  auto IsUnordered = [](MachineMemOperand *MMO) { return MMO->isUnordered(); };
-  return llvm::all_of(MI->memoperands(), IsUnordered);
+  // TODO: This should be isUnordered (see D57601) once test cases are written
+  // demonstrating that.
+  auto IsSimple = [](MachineMemOperand *MMO) {
+    return !MMO->isVolatile() && !MMO->isAtomic(); };
+  return llvm::all_of(MI->memoperands(), IsSimple);
 }
 
 ImplicitNullChecks::DependenceResult


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57802.185469.patch
Type: text/x-patch
Size: 1323 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190206/fcc3a8e2/attachment.bin>