TIFitis marked 3 inline comments as done.
TIFitis added inline comments.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4181-4191
+ 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);
----------------
jdoerfert wrote:
> TIFitis wrote:
> > I couldn't come up with a better way for the code generated inside the
> > `target data region` to use the new `AllocaPtrs` when accessing `map
> > operands` marked with the `use_device_ptr` clause.
> >
> > Currently I am just comparing the `Users` list before and after generating
> > the `data region` and replacing the new `uses`.
> >
> > Alternative method would be to use `mlir::LLVM::moduleTranslation.mapValue`
> > method to temporarily remap the `mlir::Value` for the `use_device_ptr`
> > operands to the `AllocaPtrs` `llvm::Value`. But `mapValue` doesn't allow
> > remapping of values. @kiranchandramohan Is there a good reason to prevent
> > remapping of values?
> >
> > Let me know if anyone has a better way of doing this :)
> This is only a last resort. We should not generate stuff that is wrong and is
> replaced. However, if we cannot find a better way we can talk about it.
>
> Generally, I would have assumed that the value in the region in marked to be
> a use_dev_ptr/addr and that we would generate code for that as soon as we
> generate code for the value.
I have tried to explain this a little better in https://reviews.llvm.org/D146648
The region code is generated immediately, its that it still uses the old
pointer for it instead of the newly allocated device ptr and this fixes that.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4261
+ bool Temp;
+ return Val->getPointerDereferenceableBytes(M.getDataLayout(), Temp, Temp);
}
----------------
`getPointerDereferenceableBytes` doesn't fully support `Arguments` and mostly
returns 0.
I am looking at adding support for arguments in a separate patch, in the
meantime the tests added in this patch fail because of incorrect
@.offload_sizes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146557/new/
https://reviews.llvm.org/D146557
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
