Add initial value operands to loop.parallel. The operands correspond
to the result types and loop.reduce ops in the body of the op.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks. This looks great with some minor changes addressed.
| mlir/include/mlir/Dialect/LoopOps/LoopOps.td | ||
|---|---|---|
| 238 | This confused me when I read it. The initial values are not optional. You need the same number of initial values as there are return values (which is mentioned below). | |
| mlir/lib/Dialect/LoopOps/LoopOps.cpp | ||
| 322–323 | Drop the const and & and avoid auto if the actual type is concise. for (Value init : initVals). | |
| 345–348 | I think it is preferred to not mix the two forms and just use op.emitOpError() << ... | |
| 428 | This should reflect what you actually parsed. So lower.size(), upper.size(), steps.size() and initVals.size(). | |
| mlir/lib/Dialect/LoopOps/LoopOps.cpp | ||
|---|---|---|
| 416 | nit: Can you please add /*variableName=*/ comments when using constant arguments? | |
| mlir/lib/Dialect/LoopOps/LoopOps.cpp | ||
|---|---|---|
| 413 | Nit: the comment needs an update here. | |
| 434 | Have you considered having the type closer to the init? %res = loop.parallel (%i0) = (%arg0) to (%arg1) step (%arg2) init (%zero : f32) {or %res = loop.parallel (%i0) = (%arg0) to (%arg1) step (%arg2) init (%zero) -> f32 {Having to skip visually the body is not the most readable to me. | |
| mlir/lib/Dialect/LoopOps/LoopOps.cpp | ||
|---|---|---|
| 434 | https://reviews.llvm.org/D75832 it is more readable indeed. | |
This confused me when I read it. The initial values are not optional. You need the same number of initial values as there are return values (which is mentioned below).