[PATCH] D65729: [Polly][NFC][ScopBuilder] Move buildDomains and its callees to ScopBuilder.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: polly/include/polly/ScopBuilder.h:294-300
+  /// These classes will consolidate multiple required invariant loads from
+  /// the same address in order to keep the number of dimensions in the SCoP
   /// description small. For each such class equivalence class only one
   /// representing element, hence one required invariant load, will be chosen
   /// and modeled as parameter. The method
-  /// Scop::getRepresentingInvariantLoadSCEV() will replace each element from an
-  /// equivalence class with the representing element that is modeled. As a
+  /// Scop::getRepresentingInvariantLoadSCEV() will replace each element from
+  /// an equivalence class with the representing element that is modeled. As a
----------------
[nit] unrelated noise (here and below). Possibly the result of adding and indention (e.g. by forgetting to close a `{` somewhere before these lines), running clang-format, then fixing it.


================
Comment at: polly/include/polly/ScopInfo.h:2496
+  /// Set new maximal loop depth. If @p Depth is smaller than current value,
+  /// then maximal loop dpeth is not updated.
+  void setMaxLoopDepth(unsigned Depth) {
----------------
[typo] dpeth


================
Comment at: polly/include/polly/ScopInfo.h:2497
+  /// then maximal loop dpeth is not updated.
+  void setMaxLoopDepth(unsigned Depth) {
+    MaxLoopDepth = std::max(MaxLoopDepth, Depth);
----------------
[bikeshedding] "set" might not be the right verb as it does not just overwrite the depth. How about `update` (or `notify` which you used below)?


================
Comment at: polly/include/polly/ScopInfo.h:2576
+  /// Return the domain of @p BB. If it does not exist, create an empty one.
+  isl::set &findAndConstructDomain(BasicBlock *BB) { return DomainMap[BB]; }
+
----------------
[bikeshedding] The idiom usually is named `getOr...` (e.g. `LLVMContext::getOrAddScopeRecordIdxEntry`, `Scop::getOrCreateScopArrayInfo`). How about `getOrInitEmptyDomain`?


Repository:
  rPLO Polly

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65729/new/

https://reviews.llvm.org/D65729