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

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1491
+            mapTypes = exitDataOp.getMapTypes();
+            mapperFunc = false;
+            return success();
----------------
This line is not needed after the fix you pushed right?

Seems like we could also just set `bool mapperFunc = isa<omp::EnterDataOp>(op);` or something like that.


================
Comment at: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1533
+        ompLoc, builder.saveIP(), mapTypeFlags, mapNames, mapperAllocas,
+        mapperFunc, deviceID, ifCond, processMapOpCB, bodyCB));
+  } else {
----------------
TIFitis wrote:
> TIFitis wrote:
> > mehdi_amini wrote:
> > > mapperFunc is used uninitialized here which is UB, can you look into this?
> > Thanks for pointing out, I'll push a patch to fix this.
> https://github.com/llvm/llvm-project/commit/9ea3fcfa380c6097fddd0d9a9b2c13f0f20bc41a
> 
> This fixes it.
Thanks for the quick fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914