[PATCH] D137470: [Offloading] Initial support for registering offloading entries on COFF targets

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D137470#3928731 <https://reviews.llvm.org/D137470#3928731>, @jhuber6 wrote:

> Another significant portion of getting this workflow to work for Windows / COFF is parsing the linker arguments. I should be able to look at `lld-link` and add necessarily aliases to what `ld.lld` takes I assume? E.g. we use values like `-o` and `-L` in the linker wrapper to set the output and find libraries.

Sorry, I'm not quite up to speed with exactly what is being done linker-wise here - can you give a more detailed overview? Keep in mind that there's two separate interfaces to lld for COFF; when used in mingw mode, it invokes the `ld.lld` frontend, but with a `-m` option which directs lld to the mingw frontend, which parses `ld.lld` style options and rewrites them to `lld-link` style options and invokes that interface. And when Clang is operating in msvc/clang-cl mode, `lld-link` is invoked (or called directly by the build system).



================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:145
+  // For COFF targets, sections with 8 or fewer characters containing a '$' will
+  // be merged into the same section at runtime. The order is determined by the
+  // alphebetical ordering of the text after the '$' character. Here we generate
----------------
jhuber6 wrote:
> mstorsjo wrote:
> > FWIW, this comment doesn't feel entirely accurate: Regardless of the length of the section name, all sections with names of the form `name$suffix` will get merged into the same section `name` (sorted by the suffix). Then if `name` is 8 chars or less, the name is kept in the section table (so that it can easily be looked up at runtime), while if it is longer, the full name is kept in the string table (which is not mapped at runtime).
> > 
> > Also as an extra side note; we added an exception into lld for `.eh_frame` - this is 9 chars, but libunwind wants to locate the section at runtime. So for that case, lld truncates it to `.eh_fram`. (This behaviour is lld specific, to appease libunwind - binutils doesn't do that, and libgcc locates that section differently.)
> I see, I'm not that familiar with the inner workings of the COFF linking process. All that matters for this use-case is whether or not we can get a pointer to the array. In that case we shouldn't need to worry about the eight character limit right?
If you locate the contents at runtime by using specific symbols that point to the start and end of the data, then yes, you don't need to worry about keeping it below the 8 char limit.

The 8 char limit is relevant if you enumerate and iterate over the sections of a DLL/EXE at runtime, and try to locate the section dynamically that way.


================
Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:337
-      M, DummyInit->getType(), true, GlobalVariable::ExternalLinkage, DummyInit,
-      IsHIP ? "__dummy.hip_offloading.entry" : "__dummy.cuda_offloading.entry");
-  DummyEntry->setVisibility(GlobalValue::HiddenVisibility);
----------------
jhuber6 wrote:
> mstorsjo wrote:
> > I don't quite see where the corresponding GlobalVariable for this case is created after the refactoring?
> The CUDA / HIP cases did this separately. This patch merged it into a common method `getELFEntriesArray`. Functionally this just changed the order in the output slightly. The `dummy` variable is only necessary for the ELF linkers to generate the begin / end section. For COFF we make the `$a` and `$z` variables which perform a similar role.
Ah, now I see - this is the second half of what's being merged into the cuda/hip call below in createRegisterGlobalsFunction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137470