[MLIR] adding affine parallel conversion to scf parallel in lower affine pass
Nit: use backticks instead of quotes.
This is all lacking comments - please add some.
The test cases are insufficient - please include one with non-trivial affine map bounds.
Can you please update the commit title to "[MLIR] Add lowering for affine.parallel to scf.parallel"? (Updating the commit via arc won't automatically update the title here - please hit edit here to update as well.)
Please address the comments before submitting.
@bondhugula, sorry, I thought this was relanding of a rolled back change
Nit: upperBoundValue sounds like it's a _single_ value whereas is a vector of values. This got me confused down the line where the loop is created from upperBoundValue, lowerBoundValue (singular) and steps (plural).
Please call steps.reserve before calling push_back in a loop when you know the number of elements to be inserted.
Nit: would it be possible to construct a ConstantOp and pass it an attribute without extracting its value? Something like rewriter.create<ConstantOp>(loc, step)? Loop bounds are already of index type.
+1. Also for @dot, I assumed it was a dot product.
Actually, do you expect it to never fail (it will fail on quasi-affine maps, for example)? Otherwise, you can just return failure() from the pattern.
Did you mean quasi-affine or semi-affine? expandAffineMap returns an Optional but doesn't document when it is expected to have a value nor is it obvious from either the declaration or even immediately from the implementation! Goes to show how important documentation is! Do you know who the author of this is - could we have the doc comment updated? Return values like these must be documented without fail.
In theory, we could expand any affine map, but I'm guessing semi-affine maps (where the RHS of the floordiv/ceildiv/mod is a symbol) are perhaps unsupported by expandAffineMap and so this would fail in those cases. So, for completeness sake, we should return failure for now and add a comment that in theory, it's always guaranteed to succeed if expandAffineMap is extended.