kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: jplehr.
LGTM. Please wait a day before submission. Move the flang portion to a separate
commit.
================
Comment at: flang/lib/Lower/OpenMP.cpp:727
for (mlir::Value mapOp : mapOperand) {
+ auto checkType = mapOp.getType();
+ if (auto refType = checkType.dyn_cast<fir::ReferenceType>())
----------------
kiranchandramohan wrote:
> These changes should go in a separate patch.
Nit: expand auto.
================
Comment at: flang/lib/Lower/OpenMP.cpp:727-738
+ auto checkType = mapOp.getType();
+ if (auto refType = checkType.dyn_cast<fir::ReferenceType>())
+ checkType = refType.getElementType();
+ if (auto boxType = checkType.dyn_cast<fir::BoxType>())
+ checkType = boxType.getElementType();
+
+ if (!(fir::isa_std_type(checkType) ||
----------------
These changes should go in a separate patch.
================
Comment at: flang/lib/Lower/OpenMP.cpp:737
+ !checkType.cast<fir::SequenceType>().hasDynamicExtents())))
+ TODO(currentLocation, "OMPD_target_data MapOperand type not
supported");
+
----------------
Since TODO will print a "Not yet implemented" message, I think we can remove
the trailing "not supported suffix".
================
Comment at:
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1523
+ auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
+ // DataOp has only one region associated with it.
+ auto ®ion = cast<omp::DataOp>(op).getRegion();
----------------
Nit: I think this should be an assertion.
================
Comment at:
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1502-1510
+ llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
+ llvm::OpenMPIRBuilder::InsertPointTy allocaIP =
+ findAllocaInsertPoint(builder, moduleTranslation);
+
+ struct llvm::OpenMPIRBuilder::MapperAllocas mapperAllocas;
+ SmallVector<uint64_t> mapTypeFlags;
+ SmallVector<llvm::Constant *> mapNames;
----------------
TIFitis wrote:
> kiranchandramohan wrote:
> > Can all this go into the OpenMP IRBuilder? And be passed back if necessary
> > via the callback.
> If we decide to move processMapOperand then these can be moved along with it.
Now that we decided not to move `processMapOperand`, can this be moved to the
OpenMPIRBuilder?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142914/new/
https://reviews.llvm.org/D142914
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
