[PATCH] D76700: [MLIR] Introduce full/partial tile separation using if/else

[Andy Davis via Phabricator via llvm-commits]

andydavis1 added inline comments.


================
Comment at: mlir/include/mlir/Analysis/AffineStructures.h:508
+  /// coefficient for) for identifiers in the range [pos, pos + num).
+  void removeIndConstraints(unsigned pos, unsigned num);
+
----------------
"Ind" is a bit too mysterious. How about "removeIndependentConstraints" or "removeIndepConstraints".


================
Comment at: mlir/lib/Analysis/AffineStructures.cpp:1238
+    return;
+  for (unsigned r = 0, e = getNumEqualities(); r < e; r++) {
+    if (atEq(r, pos) == 0)
----------------
Document what this block of code does.


================
Comment at: mlir/lib/Analysis/AffineStructures.cpp:1242
+    unsigned c, f;
+    for (c = offset, f = offset + num; c < f; ++c) {
+      if (c == pos)
----------------
Can this loop be moved out into a findEqWithNonZero helper function or lambda?


================
Comment at: mlir/lib/Analysis/AffineStructures.cpp:1904
+            inequalities.begin() + outputIndex);
+  inequalities.resize(inequalities.size() - numReservedCols);
+}
----------------
Add this before the resize:
assert(inequalities.size() >= numReservedCols); 


================
Comment at: mlir/lib/Analysis/AffineStructures.cpp:2801
+  // Convert to AffineExpr (tree) form.
+  auto boundExpr = getAffineExprFromFlatForm(bound, numDims - 1, numSyms,
+                                             localExprs, context);
----------------
assert(NumDims >= 1)


================
Comment at: mlir/lib/Transforms/Utils/LoopUtils.cpp:2135
+
+  // Add the body of the full tile loop.
+  BlockAndValueMapping operandMap;
----------------
This function is kind of long. Would make sense to break it in two, and start the bottom half here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76700