[PATCH] D87712: [mlir][openacc] Change operand type from index to AnyInteger in parallel op
Valentin Clement via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 17 07:26:22 PDT 2020
clementval added inline comments.
================
Comment at: mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td:84
- let arguments = (ins Optional<Index>:$async,
- Variadic<Index>:$waitOperands,
- Optional<Index>:$numGangs,
- Optional<Index>:$numWorkers,
- Optional<Index>:$vectorLength,
+ let arguments = (ins Optional<AnyInteger>:$async,
+ Variadic<AnyInteger>:$waitOperands,
----------------
ftynse wrote:
> clementval wrote:
> > clementval wrote:
> > > ftynse wrote:
> > > > Make sure `index` is also supported. It was intended as a platform-specific opaque size type. While it's reasonable to adhere to the spec and allow any integer, most lowerings coming from higher levels and many standard ops (e.g. taking the size of some container) will use `index`.
> > > I update this to use `AnyTypeOf<[AnyInteger, Index]>`
> > If that's fine I'll make a follow up patch to use the same type in `acc.loop`
> Yes, thanks!
>
> You may want to define `def IntOrIndex : AnyTypeOf<[...]>;` and use it instead of repeating every time.
Sure makes sense. Patch is updated.
================
Comment at: mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp:110
+ Type type;
+ hasOptional = false;
+ if (succeeded(parser.parseOptionalKeyword(keyword))) {
----------------
ftynse wrote:
> We have OptionalParseResult for this.
Thanks for the pointer. I was not aware of it. I updated this function and I'll update the reset in a follow up patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87712/new/
https://reviews.llvm.org/D87712
More information about the llvm-commits
mailing list