[PATCH] [Polly] Allow to annotate alias scopes in the new SCoP.
Tobias Grosser
tobias at grosser.es
Wed Oct 1 23:50:23 PDT 2014
Nice!
Some minor comments inlined.
================
Comment at: include/polly/CodeGen/CodeGeneration.h:41
@@ -39,1 +40,3 @@
+extern bool PollyAnnotateAliasScopes;
+
static inline int getNumberOfIterations(__isl_take isl_set *Domain) {
----------------
I don't think we need a command line option for this. That seems pure goodness, so why don't we enable it by default?
================
Comment at: include/polly/CodeGen/IRBuilder.h:32
@@ +31,3 @@
+/// 1) Loops are stored in a stack-like structure will keep track of all
+/// of them. Contained memory instructions and loop headers are annotated
+/// according to all parallel surrounding loops.
----------------
The first sentence talks more about the way the LoopAnnotator works and also seems somehow incomplete.
================
Comment at: lib/CodeGen/IRBuilder.cpp:34
@@ +33,3 @@
+static MDNode *getID(LLVMContext &Ctx, Value *arg0 = nullptr,
+ Value *arg1 = nullptr) {
+ MDNode *ID;
----------------
What are arg0 and arg0? Are you saying this metadata node can (optionally) reference two other metadata nodes? Maybe add this to the example in the documentation.
================
Comment at: lib/CodeGen/IRBuilder.cpp:47
@@ +46,3 @@
+ ID = MDNode::get(Ctx, Args);
+ }
+ ID->replaceOperandWith(0, ID);
----------------
What about writing this like:
```
std::vector<Value *> Args = {0}
if (arg0)
Args.push_back(arg0);
if (arg1)
Args.push_back(arg1);
ID = MDNode::get(Ctx, Args);
```
================
Comment at: lib/CodeGen/IRBuilder.cpp:63
@@ +62,3 @@
+
+ SmallPtrSet<Value *, 8> BasePtrs;
+ for (ScopStmt *Stmt : S)
----------------
Please use a data structure which guarantees the order in which we iterate over (SetVector?)
================
Comment at: lib/CodeGen/IRBuilder.cpp:135
@@ +134,3 @@
+ BasePtr = SU->getValue();
+ }
+
----------------
Would it be better to use the new to (later) use the newly introduced ScopArrayInfo class to get the base pointer?
================
Comment at: lib/CodeGen/IslCodeGeneration.cpp:610
@@ +609,3 @@
+ if (PollyAnnotateAliasScopes)
+ Annotator.buildAliasScopes(S);
+
----------------
I would enable this by default.
http://reviews.llvm.org/D5563
More information about the llvm-commits
mailing list