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

Wed Feb 22 06:59:29 PST 2023

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

Please add tests for the MLIR portion.

Could you also post the full LLVM IR for a construct with the map clause?

================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4157
+Value *OpenMPIRBuilder::getSizeInBytes(Value *BasePtr) {
+
+  LLVMContext &Ctx = Builder.getContext();
----------------
Nit:Remove empty line.

================
Comment at: mlir/lib/Target/LLVMIR/Dialect/Utils.cpp:1
+//===- Utils.cpp - General Utils for translating MLIR dialect to LLVM IR---===//
+//
----------------
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. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914