[PATCH] D146648: [MLIR][OpenMP] Added OMPIRBuilder support for use_device_ptr clause
Akash Banerjee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 22 11:29:42 PDT 2023
TIFitis added inline comments.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4070-4071
+
+ for (auto &UseDeviceOp : UseDeviceInfos)
+ UseDeviceOp.second.AllocaPtr = Builder.CreateAlloca(Builder.getPtrTy());
+
----------------
`use_dev_ptr` operands must always be pointer types so I think it is safe to always just create a `ptr` for them.
However if we want to support typed pointers then we would have to create something like:
`Builder.CreateAlloca(UseDeviceOp.first->getType()->getPointerElementType())`
This is not possible as `getPointerElementType` is deprecated and I couldn't find any alternative for this.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4180-4190
+ for (auto &UseDeviceOp : UseDeviceInfos) {
+ auto *UseDeviceVal = UseDeviceOp.first;
+ auto &UseDeviceInfo = UseDeviceOp.second;
+ SmallDenseSet<Use *> ReplaceUses;
+ for (auto &Use : UseDeviceVal->uses()) {
+ if (UseDeviceInfo.Uses.find(&Use) == UseDeviceInfo.Uses.end())
+ ReplaceUses.insert(&Use);
----------------
I have commented out some code in OpenMPToLLVMIRTranslation.cpp::1489. I think that is a better way of generating code for the use_dev_ptr, but the mapValue doesn't allow remapping.
This I am stuck with letting it use the old value inside the region and replacing it later here.
================
Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:5048-5050
Builder.restoreIP(OMPBuilder.createTargetData(
Loc, AllocaIP, Builder.saveIP(), /* IsBegin= */ false, DeviceID,
+ /* IfCond= */ nullptr, MapInfos, UseDeviceInfos, BodyCB));
----------------
This unit test fails because of opaque pointer mismatch, The unit test runs with opaque pointers disabled however the my code only runs with it enabled.
I could not find a reasonable way to support typed pointers, I have explained this in another comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146648/new/
https://reviews.llvm.org/D146648
More information about the llvm-commits
mailing list