[PATCH] D48162: [GSoC] Schedule tree performance.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: include/polly/ScheduleOptimizer.h:34-38
+    SINGLE_ELEMENT,
+    CHUNK,
+    NEIGHBOURHOOD,
+    FULL,
+    SHARED,
----------------
[Style] There is a [[ https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | coding Standard ]] on how enum members should be named.


================
Comment at: lib/Transform/ScheduleOptimizer.cpp:1337
+ 
+  for(std::size_t i = 1; i < References.size(); ++i) {
+    if(!std::equal(References[0].LoopOrder.begin(),References[0].LoopOrder.end(),
----------------
[style] [[https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop|Don't eveluate `.size()` every time through a loop]]


================
Comment at: lib/Transform/ScheduleOptimizer.cpp:1347-1353
+/// @param Node. Check if the node contains a streaming kernel.
+/// @param SK. The Skeleton array containing all the references for all the Stmts.
+/// 1. The skeleton should look like: CHUNK -> CHUNK or ELEMENT -> ELEMENT.
+/// 2. The partial index order should be the same.
+/// 3. TODO:It should have a stride in the innermost dimension.
+/// 4. TODO: Dependencies?
+
----------------
[style] Please make the doxygen comments look like the ones we already have in Polly.


================
Comment at: lib/Transform/ScheduleOptimizer.cpp:1382
 
+  /// avoid tiling.
+  if(isStreaming(isl::manage_copy(Node), OAI->Skeletons)) {
----------------
[nit] `///` is a doxygen comment, use `//` for normal comments.


================
Comment at: lib/Transform/ScheduleOptimizer.cpp:1384
+  if(isStreaming(isl::manage_copy(Node), OAI->Skeletons)) {
+    DEBUG(dbgs() << "Streaming detected\n");
+    return Node;
----------------
[style] `DEBUG` has been replaced recently by `LLVM_DEBUG`.


================
Comment at: lib/Transform/ScheduleOptimizer.cpp:1759
+
+static void printStructure(std::vector<Reference> &R, const char *StmtBaseName) {
+  assert(!R.empty() && "Structure is empty");
----------------
bollu wrote:
> This actually seems useful to have :) I am not sure what our policy is on keeping around debug printing functions, so ping @grosser @Meinersbur @philip.pfaffe and @pollydev
See `LLVM_DUMP_METHOD` macro, e.g. used by `ScopInfo::dump()`

There are more in `ISLTools.cpp`: `polly::dumpPw`


https://reviews.llvm.org/D48162