[mlir] [flang] [llvm] [clang] [MLIR][LLVM] Add Continuous Loop Peeling transform to SCF (PR #77328)

2024-01-10 Thread Matthias Springer via cfe-commits


@@ -105,6 +106,161 @@ static void specializeForLoopForUnrolling(ForOp op) {
   op.erase();
 }
 
+/// Create a new for loop for the remaining iterations (partialIteration)
+/// after a for loop has been peeled. This is followed by correcting the
+/// loop bounds for both loops given the index (splitBound) where the
+/// iteration space is to be split up. Returns failure if the loop can not
+/// be split and no new partialIteration is created.
+static LogicalResult splitLoopHelper(RewriterBase &b, scf::ForOp forOp,
+ scf::ForOp &partialIteration,
+ Value splitBound) {
+  RewriterBase::InsertionGuard guard(b);
+  auto lbInt = getConstantIntValue(forOp.getLowerBound());
+  auto ubInt = getConstantIntValue(forOp.getUpperBound());
+  auto stepInt = getConstantIntValue(forOp.getStep());
+
+  // No specialization necessary if step already divides upper bound evenly.
+  if (lbInt && ubInt && stepInt && (*ubInt - *lbInt) % *stepInt == 0)
+return failure();
+  // No specialization necessary if step size is 1.
+  if (stepInt == static_cast(1))
+return failure();
+
+  // Create ForOp for partial iteration.
+  b.setInsertionPointAfter(forOp);
+  IRMapping map;
+  auto constStepOp =
+  b.create(forOp.getLoc(), *stepInt / 2);
+  // The new for loop for the remaining iterations has half the step size
+  // as continuous peeling requires the step size to diminish exponentially
+  // across subsequent loops.
+  map.map(forOp.getStep(), constStepOp);

matthias-springer wrote:

I think this won't work. The SSA value of `forOp.getStep()` could be used in 
different ways inside of the loop and you don't want to change that.

E.g.:
```mlir
scf.for ... step %c16 {
  // This op should not be changed as part of loop peeling.
  "test.foo"(%c16) : () -> ()
}
```

What's the purpose of this `map.map`? Is it meant to canonicalize 
`affine.min/max` ops, taking into account the fact that the loop was peeled?


https://github.com/llvm/llvm-project/pull/77328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[mlir] [flang] [llvm] [clang] [MLIR][LLVM] Add Continuous Loop Peeling transform to SCF (PR #77328)

2024-01-10 Thread Matthias Springer via cfe-commits

matthias-springer wrote:

Can you re-open the old PR and force-push the contents of this PR to the old 
PR? Ideally, we'd keep using the old PR, so that we don't loose the review 
comments.


https://github.com/llvm/llvm-project/pull/77328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[mlir] [flang] [llvm] [clang] [MLIR][LLVM] Add Continuous Loop Peeling transform to SCF (PR #77328)

2024-01-10 Thread via cfe-commits

muneebkhan85 wrote:

@matthias-springer I had to move the pull request 
https://github.com/llvm/llvm-project/pull/71555 here due to an erroneous force 
push.

I have addressed all the comments you had pointed out in the original review. 
Most importantly 

1) I have re-written the logic for the comment 
https://github.com/llvm/llvm-project/pull/71555#discussion_r1444714750
 so that instead of rewriting ops after running `rewritePeeledMinMaxOp`, I 
instead correct the loop step in the map at the time the new loop is being 
created (by cloning) inside `splitLoopHelper`. This also means that there's no 
need for a change to the return type of `rewritePeeledMinMaxOp `as I had 
committed earlier. If this looks good to you, I can revert the changes to the 
return type of `rewritePeeledMinMaxOp`.

2) I have added a new test case, so that both cases `usePowerSplit = false` and 
`usePowerSplit = true` are tested 
https://github.com/llvm/llvm-project/pull/71555#discussion_r1444699098



https://github.com/llvm/llvm-project/pull/77328
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits