Be conservative about unordered accesses for the moment

[reames]

Summary

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 lands, I will return to these call sites, revert this patch, and add the appropriate tests to show the expected behaviour.

[reames]

ping

[jlebar]

ping

Strangely this didn't show up in my inbox until you pinged it. Not sure why. :-/

llvm/trunk/lib/CodeGen/ImplicitNullChecks.cpp

bool ImplicitNullChecks::canHandle(const MachineInstr *MI) {
   if (MI->isCall() || MI->hasUnmodeledSideEffects())
     return false;
   auto IsRegMask = [](const MachineOperand &MO) { return MO.isRegMask(); };
   (void)IsRegMask;
 
   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
 ImplicitNullChecks::computeDependence(const MachineInstr *MI,
                                       ArrayRef<MachineInstr *> Block) {
   assert(llvm::all_of(Block, canHandle) && "Check this first!");
   assert(!is_contained(Block, MI) && "Block must be exclusive of MI!");
 

llvm/trunk/lib/CodeGen/MachineInstr.cpp

if (!mayStore() &&
     return false;
 
   // Otherwise, if the instruction has no memory reference information,
   // conservatively assume it wasn't preserved.
   if (memoperands_empty())
     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();
   });
 }
 
 /// isDereferenceableInvariantLoad - Return true if this instruction will never
 /// trap and is loading from a location whose value is invariant across a run of
 /// this function.
 bool MachineInstr::isDereferenceableInvariantLoad(AliasAnalysis *AA) const {
   // If the instruction doesn't load at all, it isn't an invariant load.