[PATCH] [Refactor] Have getNonLocalPointerDependency take the query instruction

Philip Reames listmail at philipreames.com
Thu Jan 8 15:59:34 PST 2015


Thanks for the comments.  I'll fix and submit.


================
Comment at: include/llvm/Analysis/MemoryDependenceAnalysis.h:373
@@ -371,2 +372,3 @@
     ///
     /// This method assumes the pointer has a "NonLocal" dependency within BB.
+    void getNonLocalPointerDependency(Instruction *QueryInst,
----------------
sanjoy wrote:
> What is `BB` here?
Fixed.

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:866
@@ +865,3 @@
+  auto getLocation = [](AliasAnalysis *AA, Instruction *Inst) {
+    if (auto *I = dyn_cast<LoadInst>(Inst))
+      return AA->getLocation(I);
----------------
sanjoy wrote:
> Minor & optional: I'd just `assert(isa<LoadInst>(I) || isa<StoreInst>(I) ...)` and then `return AA->getLocation(I);` because that makes it obvious that the result is always `AA->getLocation(I)`.
AA::getLocation(Instruction *I) doesn't exists.  I may move this lambda to be that function, but that would be a separate change.

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:899
@@ +898,3 @@
+  // TODO: Handle ordered instructions
+  auto is_ordered = [](Instruction *Inst) {
+    if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
----------------
sanjoy wrote:
> Shouldn't this be `isOrdered`?  Or does llvm allow non camelcase for lambdas?
Not sure actually, but I took your suggestion.

================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:908
@@ +907,3 @@
+
+  if (is_ordered(QueryInst)) {
+    Result.push_back(NonLocalDepResult(FromBB,
----------------
sanjoy wrote:
> Where did this come from?  Is this a semantic difference?
Technically, yes.  I'll replace it with the assert - which is not a change - before submitting.  My next change is to pull in some of the logic from MemDepPrinter into this function.

http://reviews.llvm.org/D6886

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list