[PATCH] D36243: [GPGPU] Make sure that all parameter dimensions are set in schedule

[Siddharth via Phabricator via llvm-commits]

bollu added a comment.

@grosser - Can you comment on my concern about Fortran arrays? See `ScopArrayInfo::setAndApplyFAD` for this new parameter synthesis.



================
Comment at: include/polly/ScopInfo.h:2519
+  /// Returns the set of context parameters that are currently constrained. In
+  /// case the full set of parameters is needed
   __isl_give isl_space *getParamSpace() const;
----------------
- Nit: In case the full set of parameters is needed, ** see @getFullParamSpace ** (bold missing?)


================
Comment at: include/polly/ScopInfo.h:2525
+  /// getParamSpace will only return the parameters of the context that are
+  /// actually constrained, whereas getFullParamSpace will return all parameter.
+  /// This is useful in cases, where we need to ensure all parameters are
----------------
- Nit: all parameter **s**. (bold missing?)


================
Comment at: lib/Analysis/ScopInfo.cpp:4311
 
+isl::space Scop::getFullParamSpace() const {
+  isl::space Space = isl::space::params_alloc(getIslCtx(), ParameterIds.size());
----------------
I am not sure if only taking the `SCEV`s are enough. IIRC, if an array is a fortran array, it can have an extra "virtual parameter" in the outermost dimension that has no corresponding `SCEV`. That's a corner case.


================
Comment at: lib/Analysis/ScopInfo.cpp:4315
+  unsigned PDim = 0;
+  for (const auto *Parameter : Parameters) {
+    isl::id Id = isl::manage(getIdForParam(Parameter));
----------------
Nit: not sure if `auto` is valid here, because I had to lookup that `Parameter` would be a `const SCEV *`


https://reviews.llvm.org/D36243