[PATCH] D24716: [Polly] DeLICM/DePRE (WIP)

[Michael Kruse via llvm-commits]

Meinersbur added inline comments.

================
Comment at: include/polly/Support/GICHelper.h:33-34
@@ -30,1 +32,4 @@
 
+char *isl_mat_to_str(__isl_keep isl_mat *mat);
+char *isl_id_to_str(__isl_keep isl_id *mat);
+
----------------
These functions are not declared in ISL. I will upstream a patch to ISL which adds `isl_id_to_str`. `isl_mat` is not needed anymore and will be removed in an update.

================
Comment at: include/polly/Support/GICHelper.h:176
@@ -171,1 +175,3 @@
+                                 const std::string &Suffix,
+                                 bool IncludeType = false);
 
----------------
Including the type might not be necessary. I will remove that part on request.

================
Comment at: include/polly/Support/GICHelper.h:273
@@ +272,3 @@
+  ~IslPtr() {
+    if (Obj)
+      Traits::free(Obj);
----------------
This is to allow the compiler to remove the call the `Traits::free` if it knows that `Obj` is always nullptr (eg. because there is an unconditional call to `take()` before). I will commit this separately.

================
Comment at: include/polly/Support/GICHelper.h:355
@@ -336,2 +354,3 @@
   std::string toStr() const { return Traits::to_str(Obj); }
+  void dump() const { llvm::outs() << toStr() << "\n"; }
 };
----------------
Will be removed.

================
Comment at: lib/Analysis/ScopInfo.cpp:915
@@ -914,1 +914,3 @@
 
+llvm::raw_ostream &polly::operator<<(llvm::raw_ostream &OS,
+                                     const MemoryAccess &MA) {
----------------
The function returns a one-line description of a MemoryAccess. Useful eg. for
```
DEBUG(dbgs() << "Read access: " << *RA);
DEBUG(dbgs() << "Write access: " << *WA);
```

================
Comment at: lib/Analysis/ScopInfo.cpp:1119
@@ -1063,3 +1118,3 @@
   isl_map_free(NewAccessRelation);
-  NewAccessRelation = NewAccess;
+  NewAccessRelation = isl_map_align_params(NewAccess, OriginalDomainSpace);
 }
----------------
ISL removes parameters from computed schedules when they are unused. This ensures that `NewAccessRelation` has all parameters available like before. Missing parameters can cause ISL errors further down in the pipeline.

================
Comment at: lib/Support/GICHelper.cpp:180-182
@@ -179,2 +179,5 @@
   replace(str, "=>", "TO");
+  replace(str, "+", ")");
+  replace(str, "%", "");
+  replace(str, "@", "");
 }
----------------
Adding the type to ISL names causes `%` and `@` to be in the middle of the string. `+` appears in `llvm::Value`s of floating-point constants.

================
Comment at: test/ScopInfo/NonAffine/non_affine_loop_used_later.ll:4-7
@@ -3,6 +3,2 @@
 ; RUN:     -polly-allow-nonaffine-loops -analyze < %s | FileCheck %s
-; RUN: opt %loadPolly -polly-scops -polly-allow-nonaffine \
-; RUN:     -polly-process-unprofitable=false \
-; RUN:     -polly-allow-nonaffine-branches -polly-allow-nonaffine-loops \
-; RUN:     -analyze < %s | FileCheck %s --check-prefix=PROFIT
 ;
----------------
This checks the non-profitability of statements with only scalar dependencies. DeLICM might map such scalars to arrays, therefore this heuristic has been deactivated.


https://reviews.llvm.org/D24716