[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