ABataev marked an inline comment as done. ABataev added a comment. In D65819#1627631 <https://reviews.llvm.org/D65819#1627631>, @Hahnfeld wrote:
> In D65819#1627620 <https://reviews.llvm.org/D65819#1627620>, @ABataev wrote: > > > In D65819#1627600 <https://reviews.llvm.org/D65819#1627600>, @Hahnfeld > > wrote: > > > > > In D65819#1627036 <https://reviews.llvm.org/D65819#1627036>, @ABataev > > > wrote: > > > > > > > In D65819#1627032 <https://reviews.llvm.org/D65819#1627032>, @Hahnfeld > > > > wrote: > > > > > > > > > In D65819#1617736 <https://reviews.llvm.org/D65819#1617736>, > > > > > @Hahnfeld wrote: > > > > > > > > > > > Will this patch change the ability to consume a bundled object file > > > > > > without calling the unbundler? Using known ELF tools on the > > > > > > produced object files was an important design decision and IIRC was > > > > > > somewhat important for using build systems that are unaware of the > > > > > > bundled format. > > > > > > > > > > > > > > > Ping. > > > > > > > > > > > > Missed this. We still produce correct object files, so all the tools > > > > will work with this. > > > > > > > > > I agree on a technical level that it's still a "correct" object, but not > > > what I was looking for: The host object file will only be in the bundled > > > section, so you cannot examine it without unbundling. > > > > > > For example, with a small test file and `clang -fopenmp > > > -fopenmp-targets=x86_64 -c test.c` I saw the following: > > > > > > $ nm test.o > > > 0000000000000000 t .omp_offloading.requires_reg > > > 0000000000000000 T test > > > U __tgt_register_requires > > > > > > > > > After applying this patch, the output is empty which might be a problem > > > in certain cases. > > > > > > Unfortunately, this is the only possible solution I see. There are already > > 2 reports that bundled objects does not work correctly after unbundling. > > > Can you please again share what exactly is the problem, with a small example? > I saw discussions on openmp-dev, but that project was huge, and above you > were quoting a man page and hinted to global constructors. I don't have a small reproducer, unfortunately, only the big one. Here is the message from the user: Hi, I am revisiting this possible compiler bug in Clang 8.0.0. In the source code I am developing, there's a global static variable, nest::sli_neuron::recordablesMap_ put in the BSS section and it is expected to be fully initialized by the time nest::sli_neuron::sli_neuron() gets called, however in a gdb session: (gdb) p nest::sli_neuron::recordablesMap_ Python Exception <type 'exceptions.AttributeError'> 'gdb.Type' object has no attribute 'name': $1 = {<std::map<Name, double (nest::sli_neuron::*)() const, std::less<Name>, std::allocator<std::pair<Name const, double (nest::sli_neuron::*)() const> > >> = std::map with 0 elements, _vptr$RecordablesMap = 0x0} this doesn't happen when -fopenmp-targets is _not_ used, is it not trivial to come up a reproducer, thus I am sending a work-in-progress report hoping someone will shed some light on this. Thanks, Itaru. Another one: Hi, I am seeing a link error shown below: `.text.startup' referenced in section `.init_array.0' of /tmp/event_delivery_manager-02f392.o: defined in discarded section `.text.startup[_ZN4nest18DataSecondaryEventIdNS_16GapJunctionEventEE18supported_syn_ids_E]' of /tmp/event_delivery_manager-02f392.o clang-10: error: linker command failed with exit code 1 (use -v to see invocation) I am not sure how to tackle this as the part is referenced isn't what I am working on. I am using the latest trunk Clang 10 at this moment. Steps to reproduce: Steps to produce: $ git clone https://https://github.com/nest/nest-simulator/ $ mkdir /tmp/build/nest-clang-offload/ $ cd /tmp/build/nest-clang-offload/ $ cmake -DCMAKE_TOOLCHAIN_FILE=Platform/JURON_Clang -DCMAKE_INSTALL_PREFIX=/path/to/opt/nest-clang -Dwith-gsl=ON -Dwith-openmp=ON -Dwith-mpi=OFF -Dwith-python=OFF -Dwith-offload=ON /path/to/nest-simulator/ > > >> Plus, technically, we do not unbundle the original object file, so I would >> not call this unbundling at all. > > Well, after this patch unbundling is strictly required to do anything with > the host object. ================ Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:541-549 std::vector<StringRef> ClangArgs = {"clang", - "-r", + "-c", "-target", TargetName.c_str(), "-o", OutputFileNames.front().c_str(), - InputFileNames[HostInputIndex].c_str(), ---------------- Hahnfeld wrote: > ABataev wrote: > > Hahnfeld wrote: > > > Hahnfeld wrote: > > > > I think we should revert this change and just bundle the host object > > > > file as we do for all other targets. > > > To be clear: I agree that bundling + unbundling should result in the > > > exact same object file, so the other changes seem good, just having the > > > host object file easily accessible should be preserved. > > We just cannot use partial linking, it does not work for C++. > I'm only proposing to use partial linking such that external tools have easy > access to the host object. I'm fine with storing another copy of the original > host object into a section and fetch it from there during unbundling. Ahh, I see. I will try to do this. I will pack all the device objects files + host object file into a resulting bundle in the saparate sections + partial link the original host code. Will check if this works. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65819/new/ https://reviews.llvm.org/D65819 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
