[PATCH] D9888: [OPENMP] Driver support for OpenMP offloading

[Samuel Antao via cfe-commits]

sfantao added a comment.

Art, Jonas,

Thanks for the comments!

In http://reviews.llvm.org/D9888#261434, @Hahnfeld wrote:

> Currently trying to test, but
>
> 1. Offloading to the same target isn't supported (`x86_64-unknown-linux-gnu` as host and device) - this was working with `clang-omp` The produced IR isn't showing any calls to the target library and on linkage it complains: ``` undefined reference to `.omp_offloading.img_start.x86_64-unknown-linux-gnu' undefined reference to `.omp_offloading.img_end.x86_64-unknown-linux-gnu' undefined reference to `.omp_offloading.entries_begin' undefined reference to `.omp_offloading.entries_end' undefined reference to `.omp_offloading.entries_begin' undefined reference to `.omp_offloading.entries_end' ```


I assume you were trying this using the diff in http://reviews.llvm.org/D12614. There was an inconsistency in the names of the ELF sections and symbols defined by the linker script in these two patches. This is now fixed.

Note that if you are using the libomptarget library from clang-omp, you need to replace in the code `.openmptgt_host_entries` by `.omp_offloading.entries`. I changed the names so that all of them are consistent with what is already in place for other OpenMP directives.

I also changed the files generation so that different files are used even if target and host have the same triple.

Please, let me know if it still does not work for you.

> (btw: `clang-offload-bundler` saves the IR file to `$TMP` with `-S -emit-llvm`, this seems to be a bug - I had to use `--save-temps`)


Yes, the bundling job was not being marked as top level. It is now fixed!

> 2. I can't seem to figure out the target triple for NVIDIA GPUs. It should be `nvptx[64]-nvidia-cuda` which gives me ``` include/llvm/Option/Option.h:101: const llvm::opt::Option llvm::opt::Option::getAlias() const: Assertion `Info && "Must have a valid info!"' failed. ``` In `clang-omp` it was `nvptxsm_35-nvidia-cuda` but this is now invalid...


I didn't implement the triples logic for the nvptx targets yet. I'll port that from clang-omp once we have the basic functionality working upstream.

I'll address Art's comments in a separate message.

Thanks again,
Samuel


================
Comment at: include/clang/Driver/Driver.h:208
@@ +207,3 @@
+  /// CreateUnbundledOffloadingResult - Create a command to unbundle the input
+  /// and use the resulting input info. If there re inputs already cached in
+  /// OffloadingHostResults for that action use them instead. If no offloading
----------------
tra wrote:
> re -> are
Fixed!

================
Comment at: include/clang/Driver/Driver.h:210
@@ +209,3 @@
+  /// OffloadingHostResults for that action use them instead. If no offloading
+  /// is being support just return the provided input info.
+  InputInfo CreateUnbundledOffloadingResult(
----------------
tra wrote:
> "If offloading is not supported" perhaps?
Fixed!

================
Comment at: lib/Driver/Driver.cpp:2090
@@ +2089,3 @@
+          dyn_cast<OffloadUnbundlingJobAction>(A)) {
+    // The input of the unbundling job has to a single input non-source file,
+    // so we do not consider it having multiple architectures. We just use the
----------------
tra wrote:
> "has to be"
Fixed!


http://reviews.llvm.org/D9888