jhuber6 added a comment. Thanks for the feedback.
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. ================ Comment at: clang/test/CodeGenCUDA/offloading-entries.cu:92 +// CUDA-COFF-NEXT: entry: +// CUDA-COFF-NEXT: [[TMP0:%.*]] = call i32 @cudaLaunch(ptr @_Z18__device_stub__barv) +// CUDA-COFF-NEXT: br label [[SETUP_END:%.*]] ---------------- mstorsjo wrote: > Is this identical to the one above? Should the lines be shared with > `--check-prefix=COMMON,COFF` etc? (The number of lines is rather small here > so it's maybe not strictly necessary, but I saw that done in the other > testcase.) Yeah it might be valid to collapse it further, this test is mostly just copy-pasted directly from the output so we should probably try to keep it common. ================ 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 ---------------- 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? ================ 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); ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137470/new/ https://reviews.llvm.org/D137470 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
