[PATCH] [Refactor] Have getNonLocalPointerDependency take the query instruction
Sanjoy Das
sanjoy at playingwithpointers.com
Thu Jan 8 15:40:48 PST 2015
The only thing that concerns me is the new alternate return path on `if (is_ordered(QueryInst)) {`. With that addressed, LGTM.
================
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,
----------------
What is `BB` here?
================
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);
----------------
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)`.
================
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)) {
----------------
Shouldn't this be `isOrdered`? Or does llvm allow non camelcase for lambdas?
================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:908
@@ +907,3 @@
+
+ if (is_ordered(QueryInst)) {
+ Result.push_back(NonLocalDepResult(FromBB,
----------------
Where did this come from? Is this a semantic difference?
http://reviews.llvm.org/D6886
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list