[Openmp-commits] [PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via Openmp-commits]

sdmitriev marked 4 inline comments as done.
sdmitriev added inline comments.


================
Comment at: clang/include/clang/Driver/Action.h:74
     OffloadUnbundlingJobClass,
+    OffloadWrapperJobClass,
 
----------------
ABataev wrote:
> Do we really need this new kind of job here, can we use bundler instead?
Well, I can probably try to reuse bundling action for wrapping, but I think it will just complicate the logic. Wrapping logically differs from bundling and wrapping is done by a different tool, so I think it is natural to add a distinct action class for it.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9748
+  // If we have offloading in the current module, we need to emit the entries.
   createOffloadEntriesAndInfoMetadata();
 }
----------------
ABataev wrote:
> Do not emit it for the devices and simd only mode. Also, would be good to assert if no devices triples were specified.
Offload entries are actually emitted both for host and target compilations. I have added a check for OpenMP simd mode to createOffloadingBinaryDescriptorRegistration().


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:1470
+  /// was emitted in the current module.
+  virtual void emitOffloadTables();
 
----------------
ABataev wrote:
> Ithink, you can drop `virtual` here and remove overridden version from the CGOpenMPRuntimeSimd. Instead, just check for OpenMP simd mode in the original function and just early exit in this case.
Ok. Without virtual I do not see much reasons for adding new function which just calls createOffloadEntriesAndInfoMetadata(), so instead I have just made createOffloadEntriesAndInfoMetadata() public and added a check for OpenMP simd mode to this function.


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

https://reviews.llvm.org/D64943