Update ROOT's llvm to llvm13. #10294
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vgvassilev
requested review from
bellenot,
Axel-Naumann and
pcanal
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
I tested a bit on my end, I guess the This happens with a C++17 build in general, C++14 is fine. On the performance side, the current state seems to veery slow: |
I do not understand why. Calling
Can you paste the stack trace? I fear that the DynamicLibraryManagerSymbol.cpp stopped inlining functions due to some recent calls of ::StringRef::str()... |
I think the problem is that diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx
index 0900c4d62a..b288aef228 100644
--- a/core/metacling/src/TCling.cxx
+++ b/core/metacling/src/TCling.cxx
@@ -3164,10 +3164,11 @@ Bool_t TCling::IsLoaded(const char* filename) const
llvm::StringRef(filesStr).split(files, "\n");
std::set<std::string> fileMap;
+ llvm::StringRef file_name_ref(file_name);
// Fill fileMap; return early on exact match.
for (llvm::SmallVector<llvm::StringRef, 100>::const_iterator
iF = files.begin(), iE = files.end(); iF != iE; ++iF) {
- if ((*iF) == file_name.c_str()) return kTRUE; // exact match
+ if ((*iF) == file_name_ref) return kTRUE; // exact match
fileMap.insert(iF->str());
}
@@ -7116,9 +7117,12 @@ static std::string GetSharedLibImmediateDepsSlow(std::string lib,
// FIXME: It is unclear whether we can ignore all weak undefined
// symbols:
// http://lists.llvm.org/pipermail/llvm-dev/2017-October/118177.html
- if (SymName == "_Jv_RegisterClasses" ||
- SymName == "_ITM_deregisterTMCloneTable" ||
- SymName == "_ITM_registerTMCloneTable")
+ static constexpr llvm::StringRef RegisterClasses("_Jv_RegisterClasses");
+ static constexpr llvm::StringRef RegisterCloneTable("_ITM_registerTMCloneTable");
+ static constexpr llvm::StringRef DeregisterCloneTable("_ITM_deregisterTMCloneTable");
+ if (SymName == RegisterClasses ||
+ SymName == RegisterCloneTable ||
+ SymName == DeregisterCloneTable)
continue;
}
Do you want me to submit this separately, outside this PR?
|
I think you should be able to submit as part of this PR.
yeah, this is a bit tricky, the profiler is useful if you built in debug mode. What usually works is removing the My feeling is that I mechanically added |
|
The last three commits are for Cling's CUDA support. It still doesn't fully work on my machine, but the errors are the same as |
That sounds pretty good! I remember @SimeonEhrig mentioning some issues when loading the cuda library. PS: if the cuda test state is the same as it is in the master maybe we can go off hunting the root test failures and eventually come back to cuda after? |
|
Yes, I agree that we should now focus on the remaining test failures, both in Cling and ROOT. For the "file name too long" when building with GCC, I've posted #10387 (we'll have to rebase this PR afterwards and change a number of the new I also started looking into the slow JIT for RDF, and I noticed that it's completely hanging when ROOT is built with C++17. The stack trace of a stuck It looks like we have a problem with re-entrant JITing, but I have absolutely no idea if that's supposed to work or points to a problem somewhere else @vgvassilev |
Original commit message:" [ORC] Fix weak hidden symbols failure on PPC with runtimedyld Fix "JIT session error: Symbols not found: [ DW.ref.__gxx_personality_v0 ] error" which happens when trying to use exceptions on ppc linux. To do this, it expands AutoClaimSymbols option in RTDyldObjectLinkingLayer to also claim weak symbols before they are tried to be resovled. In ppc linux, DW.ref symbols is emitted as weak hidden symbols in the later stage of MC pipeline. This means when using IRLayer (i.e. LLJIT), IRLayer will not claim responsibility for such symbols and RuntimeDyld will skip defining this symbol even though it couldn't resolve corresponding external symbol. Reviewed By: sgraenitz Differential Revision: https://reviews.llvm.org/D129175 " This patch fixes the same issue for ROOT on ppc64le.
Original commit message: " Teach RuntimeDyld to handle COFF weak references and to consider comdat symbols as weak. Patch by Lang Hames and Sunho Kim! " https://reviews.llvm.org/D138264
The relocation needs to allow for long offsets, as for the JIT, __dso_handle
might be outside the shared library. Fixes
```
cling JIT session error: In graph cling-module-10-jitted-objectbuffer, section __TEXT,__StaticInit: relocation target "___dso_handle" at address 0x7fe1ee5052e0 is out of range of Delta32 fixup at 0x108c410bd (___cxx_global_var_initcling_module_10_, 0x108c41090 + 0x2d)
[runStaticInitializersOnce]: Failed to materialize symbols: { (main, { $.cling-module-10.__inits.0, __ZN12IncidentTypeL2m1E, __ZN6MarkerD2Ev, __ZN6MarkerD1Ev, ___cxx_global_var_initcling_module_10_.1, __GLOBAL__sub_I_cling_module_10, __ZN6MarkerC2EPKc, ___cxx_global_var_initcling_module_10_.3, __ZN12IncidentTypeL2m3E, __ZN6MarkerC1EPKc, __ZN12IncidentTypeL2m2E, ____orc_init_func.cling-module-10, ___cxx_global_var_initcling_module_10_ }) }
```
as seen no RISC-V and macOS, i.e. with the JITLinker.
Previously, one needed to pass linker-mangled names, which exposes details that clients of IncrementalExecutor should not have to deal with. Instead, use the IR name and do the linker-mangling in IncrementalJIT::addOrReplaceDefinition(). This fixes the lack of static destruction on macOS, visible e.g. in the test failure of roottest/cling/staticinit/ROOT-7775.
This helps with debugging why a certain header does not end up in a module that is currently generated.
It is used by core/base and causes laying violations with modules, see module ROOT_Foundation_C which needs to contain ThreadLocalStorage.h.
This is still required for proper exception handling support. See commits 3f74182 and a7b0b3e by Philippe for details on the problem and the original code, which I was able to significantly simplify with the new JIT infrastructure. For the moment, this disables JITLink even on platforms that had it active by default (notably macOS). This will be reintroduced at a later point, which will require a memory manager implementation for the JITLink interface.
This is required until CallFunc is informed about unloading, and can re-generate the wrapper (if the decl is still available).
If the Decl pointers are not identical, declaresSameEntity will check the canonical Decls. This fixes the df021_createTGraph tutorial on CentOS 8. Upstream discussion in https://reviews.llvm.org/D137787
It finds a false positive in llvm::DataLayout::reset accessing DefaultAlignments while loading the Net library. At this point, Cling and LLVM have definitely been initialized. This is a known limitation of the order check, which is why it was disabled by default. Remove our config to enable it for now to allow checking the LLVM upgrade with AddressSanitizer instrumentation.
This is only used to determine the architecture for OpenMP offloading, which we are not interested in.
As https://johannst.github.io/notes/development/symbolver.html points out, "@@" just means "default version". We want to skip any versioned GCC/glibc/ libstdc++ symbol. Significantly reduces appetite for searching symbols. Then also skip wek symbols: even if they are unresolved we will happily not have them if they have not been loaded so far. Typical case is `__gmon_start__`.
When creating orec-Symbols for dylib symbols, reloading the dylib might mean a change in symbol (address). So unloading a dylib means we need to unload the orc-Symbol. This is implemented through resource-tracking the symbols as provided by DynamicLibrarySearchGenerator. Actually, as DynamicLibrarySearchGenerator does not support resource tracking, it is implemented in a near-copy of DynamicLibrarySearchGenerator, RTDynamicLibrarySearchGenerator, which uses the transaction of the most recent module for the ResourceTracker.
The splitting code requires the full string to be null-terminated. Before, random bytes were attached to the last option.
on macOS11, stressInterpreter fails with not re-emitting _ZL6strstrUa9enable_ifILb1EEPKcS0_. It has internal linkage: originally deferred, it needs a chance to be re-emitted. Simplify this to include anything that was deferred and is either weak-for-linker or internal, to hopefully match all cases.
Axel-Naumann
approved these changes
Dec 9, 2022
There was a problem hiding this comment.
Congrats everyone, with special thanks to @vgvassilev for driving this for so many months, and @weliveindetail @lhames @SimeonEhrig and @hahnjo for invaluable help!
|
Starting build on |
This was referenced Jan 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The things we need to do before merging this PR and can probably be done by various people in parallel
Cling standalone:
Cling test failures
Failures in
masteron my system:Remaining failures (excluding the ones above):
ROOT:
.pcmfile size against masterBinary Size this PR needs 13% more space (2.3 vs 2. GB)
Module files need ~5% more space on disk (215 vs 206 MB)
cc: @hahnjo, @Axel-Naumann