[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