[PATCH] D74708: [MLIR][Ploops] Add custom builders from ParallelOp and ReduceOp.

[Stephan Herhut via Phabricator via llvm-commits]

herhut accepted this revision.
herhut added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:188
+              "ValueRange lowerBounds, ValueRange upperBounds, "
+              "ValueRange steps, ArrayRef<Type> resultTypes">
   ];
----------------
pifon2a wrote:
> mehdi_amini wrote:
> > Isn't a loop always supposed to capture as operands the same types as the returned values? If so then why do we have a builder with `resultTypes`? I'd expect these to be inferred from the captured operands?
> > 
> > It is a problem with the current reduce, which does not model this, but that's a bug to fix right? And so adding a builder around the incorrect design does not seem right to me.
> Ploops don't really capture any operands from which we can infer result types. The result types correspond to the ReduceOps in the body of the ploop.
We have not added neutral values that can be used as initial values to parallel loops, yet, and I am not sure whether they should be mandatory. Having neutral values unlocks some additional reduction schemes but they are not always necessary. If we make them optional, we will need to provide return types to support that. 
If we make them mandatory, we can adapt the builders and their use sites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74708