[REFACTOR] Push logic from MemDepPrinter into getNonLocalPointerDependency
authorPhilip Reames <listmail@philipreames.com>
Fri, 9 Jan 2015 00:26:45 +0000 (00:26 +0000)
committerPhilip Reames <listmail@philipreames.com>
Fri, 9 Jan 2015 00:26:45 +0000 (00:26 +0000)
Previously, MemDepPrinter handled volatile and unordered accesses without involving MemoryDependencyAnalysis.  By making a slight tweak to the documented interface - which is respected by both callers - we can move this responsibility to MDA for the benefit of any future callers.  This is basically just cleanup.

In the future, we may decide to extend MDA's non local dependency analysis to return useful results for ordered or volatile loads.  I believe (but have not really checked in detail) that local dependency analyis does get useful results for ordered, but not volatile, loads.

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

include/llvm/Analysis/MemoryDependenceAnalysis.h
lib/Analysis/MemDepPrinter.cpp
lib/Analysis/MemoryDependenceAnalysis.cpp

index 672c839d33bbd64ee19bd7042a89b85fbc6ee4ec..67fd70a4561fa06cdfa60afbe740a60c950c770d 100644 (file)
@@ -366,9 +366,12 @@ namespace llvm {
 
 
     /// getNonLocalPointerDependency - Perform a full dependency query for an
-    /// access to the QueryInst's specified (non-volatile) memory location,
-    /// returning the set of instructions that either define or clobber
-    /// the value.
+    /// access to the QueryInst's specified memory location, returning the set
+    /// of instructions that either define or clobber the value.
+    ///
+    /// Warning: For a volatile query instruction, the dependencies will be
+    /// accurate, and thus usable for reordering, but it is never legal to
+    /// remove the query instruction.  
     ///
     /// This method assumes the pointer has a "NonLocal" dependency within
     /// QueryInst's parent basic block.
index ed77da026902e70422431df48c4b41419cbe570e..ffc9fe64cc526f2c3f1de27f0ef446b6503bd75e 100644 (file)
@@ -118,27 +118,9 @@ bool MemDepPrinter::runOnFunction(Function &F) {
       }
     } else {
       SmallVector<NonLocalDepResult, 4> NLDI;
-      if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
-        if (!LI->isUnordered()) {
-          // FIXME: Handle atomic/volatile loads.
-          Deps[Inst].insert(std::make_pair(getInstTypePair(nullptr, Unknown),
-                                           static_cast<BasicBlock *>(nullptr)));
-          continue;
-        }
-        MDA.getNonLocalPointerDependency(LI, NLDI);
-      } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
-        if (!SI->isUnordered()) {
-          // FIXME: Handle atomic/volatile stores.
-          Deps[Inst].insert(std::make_pair(getInstTypePair(nullptr, Unknown),
-                                           static_cast<BasicBlock *>(nullptr)));
-          continue;
-        }
-        MDA.getNonLocalPointerDependency(SI, NLDI);
-      } else if (VAArgInst *VI = dyn_cast<VAArgInst>(Inst)) {
-        MDA.getNonLocalPointerDependency(VI, NLDI);
-      } else {
-        llvm_unreachable("Unknown memory instruction!");
-      }
+      assert( (isa<LoadInst>(Inst) || isa<StoreInst>(Inst) ||
+               isa<VAArgInst>(Inst)) && "Unknown memory instruction!"); 
+      MDA.getNonLocalPointerDependency(Inst, NLDI);
 
       DepSet &InstDeps = Deps[Inst];
       for (SmallVectorImpl<NonLocalDepResult>::const_iterator
index cc459482b2ce6e6bc6143fefa1604ff7456f9180..c505aa48817057c1126442c52ffb4c33fd67404b 100644 (file)
@@ -881,18 +881,23 @@ getNonLocalPointerDependency(Instruction *QueryInst,
   bool isLoad = isa<LoadInst>(QueryInst);
   BasicBlock *FromBB = QueryInst->getParent();
   assert(FromBB);
+
+  assert(Loc.Ptr->getType()->isPointerTy() &&
+         "Can't get pointer deps of a non-pointer!");
+  Result.clear();
   
-  // This routine does not expect to deal with volatile instructions.  Doing so
-  // would require piping through the QueryInst all the way through.
+  // This routine does not expect to deal with volatile instructions.
+  // Doing so would require piping through the QueryInst all the way through.
   // TODO: volatiles can't be elided, but they can be reordered with other
-  // non-volatile accesses.  
-  if (LoadInst *LI = dyn_cast<LoadInst>(QueryInst)) {
-    assert(!LI->isVolatile());
-  } else if (StoreInst *SI = dyn_cast<StoreInst>(QueryInst)) {
-    assert(!SI->isVolatile());
-  }
-
-
+  // non-volatile accesses.
+  auto isVolatile = [](Instruction *Inst) {
+    if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
+      return LI->isVolatile();
+    } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
+      return SI->isVolatile();
+    }
+    return false;
+  };
   // We currently give up on any instruction which is ordered, but we do handle
   // atomic instructions which are unordered.
   // TODO: Handle ordered instructions
@@ -904,11 +909,13 @@ getNonLocalPointerDependency(Instruction *QueryInst,
     }
     return false;
   };
-  assert(!isOrdered(QueryInst) && "ordered instructions not expected");
+  if (isVolatile(QueryInst) || isOrdered(QueryInst)) {
+    Result.push_back(NonLocalDepResult(FromBB,
+                                       MemDepResult::getUnknown(),
+                                       const_cast<Value *>(Loc.Ptr)));
+    return;
+  }
 
-  assert(Loc.Ptr->getType()->isPointerTy() &&
-         "Can't get pointer deps of a non-pointer!");
-  Result.clear();
 
   PHITransAddr Address(const_cast<Value *>(Loc.Ptr), DL, AC);