[PATCH] isDereferenceablePointer: look through gc.relocates + test
Philip Reames
listmail at philipreames.com
Fri Jan 23 11:16:46 PST 2015
I'd suggest splitting this into two patches: adding the new analysis, adding the new optimization and testing it. If you really didn't want to for some reason, it's not that big a deal, but it would make it easier to review quickly.
================
Comment at: include/llvm/IR/InstIterator.h:130
@@ -129,1 +129,3 @@
inline inst_iterator inst_end(Function *F) { return inst_iterator(*F, true); }
+inline iterator_range<inst_iterator> insts(Function *F) {
+ return iterator_range<inst_iterator>(inst_begin(F), inst_end(F));
----------------
I might call this inst_range. I'm usually not a fan of extra verbosity, but I see potential confusion resulting from a such short relatively common name being added to the llvm namespace.
================
Comment at: lib/Analysis/MemDepPrinter.cpp:147
@@ -146,6 +146,3 @@
- for (DepSet::const_iterator I = InstDeps.begin(), E = InstDeps.end();
- I != E; ++I) {
- const Instruction *DepInst = I->first.getPointer();
- DepType type = I->first.getInt();
- const BasicBlock *DepBB = I->second;
+ for (auto I: InstDeps) {
+ const Instruction *DepInst = I.first.getPointer();
----------------
Please separate this refactoring into a separate change.
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:1
@@ +1,2 @@
+//===- MemDerefPrinter.cpp - Printer for isDereferenceablePointer ---------===//
+//
----------------
Please split the introduction of the printer and one or two example tests into it's own change. I'll be happy to LGTM that one quickly.
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:22
@@ +21,3 @@
+ struct MemDerefPrinter : public FunctionPass {
+ const Function *F;
+
----------------
Why do you need to capture F?
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:24
@@ +23,3 @@
+
+ enum DepType {
+ Clobber = 0,
----------------
Why is this enum needed?
================
Comment at: lib/Analysis/MemDerefPrinter.cpp:43
@@ +42,3 @@
+ void releaseMemory() override {
+ F = nullptr;
+ }
----------------
You should also clear your small vector. (I think)
================
Comment at: lib/IR/Value.cpp:575
@@ -572,1 +574,3 @@
+ // For gc.relocate, look through relocations
+ if (const IntrinsicInst *I = dyn_cast<IntrinsicInst>(V))
----------------
I'd suggest using isGCRelocate here.
http://reviews.llvm.org/D7075
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list