[PATCH] D131915: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

Sun Jan 15 08:52:05 PST 2023

kiranchandramohan added inline comments.

================
Comment at: mlir/lib/Dialect/OpenMP/CMakeLists.txt:15
   MLIRLLVMDialect
+  MLIRArithDialect
   )
----------------
TIFitis wrote:
> kiranchandramohan wrote:
> > TIFitis wrote:
> > > kiranchandramohan wrote:
> > > > Why is this needed here?
> > > The Arith Dialect needs to be linked against as we're using it extract the int value from arith.constant in the custom printer.
> > Can this be avoided by modelling constants as attributes?
> The issue with attributes is AFAIK `Variadic<Attribute>` is not supported, and as previously discussed we need it be `Variadic` to support multiple map clauses.
> 
> If I am wrong and there is indeed a way to have `Variadic<Attribute>` then this can be avoided.
Can we use an ArrayAttr (https://mlir.llvm.org/docs/Dialects/Builtin/#arrayattr) or something derived from it?
https://github.com/llvm/llvm-project/blob/6ee5a1a090f3f4b6ae7ec9915900023277daf8d7/mlir/include/mlir/IR/OpBase.td#L1467

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131915