[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

Thu Feb 23 01:51:00 PST 2023

kiranchandramohan added a comment.

In D142914#4145224 <https://reviews.llvm.org/D142914#4145224>, @TIFitis wrote:

> In D142914#4144475 <https://reviews.llvm.org/D142914#4144475>, @kiranchandramohan wrote:
>
>> Please add tests for the MLIR portion.
>
> Can you please tell me where to add these?

In `mlir/test/Target/LLVMIR/openmp-llvm.mlir`. You can also consider starting a new file for target constructs in that directory.

================
Comment at: mlir/lib/Target/LLVMIR/Dialect/Utils.cpp:1
+//===- Utils.cpp - General Utils for translating MLIR dialect to LLVM IR---===//
+//
----------------
TIFitis wrote:
> kiranchandramohan wrote:
> > Nit: I would prefer the OpenMPCommon.cpp name suggested in https://discourse.llvm.org/t/rfc-adding-new-util-file-to-mlir-dialect-translation/68221. 
> Would you also like me to move the file inside OpenMP ( `mlir/lib/Target/LLVMIR/Dialect/OpenMP` ) ?
No, the current location is fine.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914