[PATCH] D76363: [MLIR] Add parallel loop coalescing.

[Theodore Popp via Phabricator via llvm-commits]

tpopp marked an inline comment as done.
tpopp added inline comments.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:1141
+  for (unsigned i = 0; i < combinedDimensions.size(); i++) {
+    Value newUpperBound = outsideBuilder.create<ConstantIndexOp>(loc, 1);
+    for (auto idx : combinedDimensions[i]) {
----------------
herhut wrote:
> Why not `newUpperBound = cst1` here?
No real reason. I thought it would be easier for debugging purposes if each string of calculations is fully unconnected from other calculations.


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:1161
+        previous = insideBuilder.create<SignedDivIOp>(
+            loc, previous, loops.upperBound()[ivar_idx]);
+
----------------
herhut wrote:
> Should this be the normalized upper bound?
Yes


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:1166
+                     : insideBuilder.create<SignedRemIOp>(
+                           loc, previous, loops.upperBound()[ivar_idx]);
+      replaceAllUsesInRegionWith(loops.getBody()->getArgument(ivar_idx), iv,
----------------
herhut wrote:
> Normalized here, too?
Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76363