[PATCH] D136853: [OpenMP][OMPIRBuilder] Migrate createOffloadEntriesAndInfoMetadata from clang to OpenMPIRBuilder
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 27 11:09:43 PDT 2022
jdoerfert added a comment.
I think the pure code move looks good. My comments below are more concerned with cleanup/clarification we could do while we are here.
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:27
class CanonicalLoopInfo;
+class TargetRegionEntryInfo;
class OffloadEntriesInfoManager;
----------------
It's a struct, according to the pre-commit buildbot error.
================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1101
+ uint64_t Size, int32_t Flags,
+ GlobalValue::LinkageTypes);
+
----------------
IsGPU is not great. If anything, let's call it "IsTargetCodegen" or similar. Also elsewhere. As this clashes with the special handling of "host offloading", I would suggest to then rename `IsDevice` to `IsEmbedded` or something. This should work for non-GPUs and also the "IsDevice" flag is misnamed as the host offloading is also targeting a device.
================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4734
+ if (IsSIMD || OffloadEntriesInfoManager.empty())
+ return;
+
----------------
Why do we call it in SIMD mode in the first place?
The Module should be available to the IRBuilder.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136853/new/
https://reviews.llvm.org/D136853
More information about the llvm-commits
mailing list