-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump LLVM to v19.1.1+1 #56130
base: master
Are you sure you want to change the base?
Bump LLVM to v19.1.1+1 #56130
Conversation
c1900d0
to
adc7db4
Compare
The EDIT: gcc is giving a slightly different warning on CI but likely a bug as well |
In that case you can guard the offending line with something like #pragma GCC diagnostic push
#if defined(_COMPILER_GCC_) && __GNUC__ >= 12 // if this is version-dependent
// Explain why this is being ignored...
#pragma GCC diagnostic ignored "-Wstringop-overflow"
#endif
...
#pragma GCC diagnostic pop |
Also, all tests are passing already on aarch64-darwin 🥳 |
Looks like the only two issues are a incorrect warning during the build (mose's suggestion doesn't seem to work though |
Guessing the asan problem is |
Without the push-and-pop, that becomes closer to just adding Lines 23 to 25 in 80e60c8
|
5c3c307
to
24f4d93
Compare
Looks like remaining issue is some failures in |
Second analyzegc failure looks incorrect, it seems to incorrectly think |
@nanosoldier |
Need to rebase on master now that #56133 has been merged. |
f6b2376
to
e12026f
Compare
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
FWIW, the standard way to deal with this is to add an assert and a comment, as that will make both reviewers and bots happy |
The %12 = select i1 %exactly_isa.not, float -0.000000e+00, float %immutable_union.sroa.0.0.copyload, !dbg !77
%value_phi6 = fadd float %value_phi, %12, !dbg !77 to %12 = fadd float %value_phi, %immutable_union.sroa.0.0.copyload, !dbg !77
%value_phi6 = select i1 %exactly_isa.not, float %value_phi, float %12, !dbg !77 prevents the loopvectorize pass from SIMDing the code. I suspect the other regression for the union.array benchmarks are similar. |
I'll try rebasing on top of #52850 and see if that fixes the regressions. |
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. |
@nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
looks like there are a few 8x (vectorization probably) regressions here. |
PkgEval failure for McCormick is |
c537718
to
dfec0aa
Compare
Analyzegc is still failing:
|
`clang -print-runtime-dir` reports a non-existent directory as we build with `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`. See llvm/llvm-project#102834. I suspect llvm/llvm-project@b6a1473 caused the change by chaning the code in Driver.cpp to not check whether the printed directory existed.
Locally I get: ``` In file included from /home/rag/Documents/Code/julia/usr/include/llvm/Object/ObjectFile.h:18, from /home/rag/Documents/Code/julia/usr/include/llvm/DebugInfo/DIContext.h:18, from /home/rag/Documents/Code/julia/src/debuginfo.cpp:6: In function 'bool llvm::operator==(llvm::StringRef, llvm::StringRef)', inlined from 'bool llvm::operator!=(llvm::StringRef, llvm::StringRef)' at /home/rag/Documents/Code/julia/usr/include/llvm/ADT/StringRef.h:874:71, inlined from 'objfileentry_t find_object_file(uint64_t, llvm::StringRef)' at /home/rag/Documents/Code/julia/src/debuginfo.cpp:948:43, inlined from 'bool jl_dylib_DI_for_fptr(size_t, llvm::object::SectionRef*, int64_t*, llvm::DIContext**, bool, bool*, uint64_t*, void**, char**, char**)' at /home/rag/Documents/Code/julia/src/debuginfo.cpp:1135:34: /home/rag/Documents/Code/julia/usr/include/llvm/ADT/StringRef.h:871:20: warning: 'int __builtin_memcmp_eq(const void*, const void*, long unsigned int)' specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overread] 871 | return ::memcmp(LHS.data(), RHS.data(), LHS.size()) == 0; | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/rag/Documents/Code/julia/src/debuginfo.cpp: In function 'bool jl_dylib_DI_for_fptr(size_t, llvm::object::SectionRef*, int64_t*, llvm::DIContext**, bool, bool*, uint64_t*, void**, char**, char**)': /home/rag/Documents/Code/julia/src/debuginfo.cpp:1133:11: note: source object allocated here 1133 | fname = dlinfo.dli_fname; ``` On CI: ``` In file included from /cache/build/builder-amdci4-2/julialang/julia-master/usr/include/llvm/Object/ObjectFile.h:18, from /cache/build/builder-amdci4-2/julialang/julia-master/usr/include/llvm/DebugInfo/DIContext.h:18, from /cache/build/builder-amdci4-2/julialang/julia-master/src/debuginfo.cpp:6: In function 'bool llvm::operator==(llvm::StringRef, llvm::StringRef)', inlined from 'bool llvm::operator!=(llvm::StringRef, llvm::StringRef)' at /cache/build/builder-amdci4-2/julialang/julia-master/usr/include/llvm/ADT/StringRef.h:874:71, inlined from 'objfileentry_t find_object_file(uint64_t, llvm::StringRef)' at /cache/build/builder-amdci4-2/julialang/julia-master/src/debuginfo.cpp:943:43, inlined from 'bool jl_dylib_DI_for_fptr(size_t, llvm::object::SectionRef*, int64_t*, llvm::DIContext**, bool, bool*, uint64_t*, void**, char**, char**)' at /cache/build/builder-amdci4-2/julialang/julia-master/src/debuginfo.cpp:1126:47: /cache/build/builder-amdci4-2/julialang/julia-master/usr/include/llvm/ADT/StringRef.h:871:20: error: 'int __builtin_memcmp_eq(const void*, const void*, long unsigned int)' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=] 871 | return ::memcmp(LHS.data(), RHS.data(), LHS.size()) == 0; | ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ```
The change to `GCChecker.cpp` is so the static analyzer doesn't see e.g. a call to a function pointer in llvm and then complain that it might be a safepoint.
e7698a13e319a9919af04d3d693a6f6ea7168a44 isn't in llvm 19
dd978af
to
71f9f18
Compare
@nanosoldier |
The package evaluation job you requested has completed - possible new issues were detected. |
On this PR julia> code_llvm(+, NTuple{2,Float64}; debuginfo=:none) ; Function Signature: +(Float64, Float64)
define double @"julia_+_1011"(double %"x::Float64", double %"y::Float64") #0 {
top:
#dbg_value(double %"x::Float64", !3, !DIExpression(), !16)
#dbg_value(double %"y::Float64", !15, !DIExpression(), !16)
%0 = fadd double %"x::Float64", %"y::Float64"
ret double %0
} Looks like the syntax of the debug values was changed in LLVM 19. |
Other issue related to native/llvm code printing. On julia> code_native(+, NTuple{2,Float64}; debuginfo=:none) .text
.file "+"
.section .ltext,"axl",@progbits
.globl "julia_+_738" # -- Begin function julia_+_738
.p2align 4, 0x90
.type "julia_+_738",@function
"julia_+_738": # @"julia_+_738"
; Function Signature: +(Float64, Float64)
# %bb.0: # %top
#DEBUG_VALUE: +:x <- $xmm0
#DEBUG_VALUE: +:y <- $xmm1
push rbp
mov rbp, rsp
vaddsd xmm0, xmm0, xmm1
pop rbp
ret
.Lfunc_end0:
.size "julia_+_738", .Lfunc_end0-"julia_+_738"
# -- End function
.type ".L+Core.Float64#740",@object # @"+Core.Float64#740"
.section .lrodata,"al",@progbits
.p2align 3, 0x0
".L+Core.Float64#740":
.quad ".L+Core.Float64#740.jit"
.size ".L+Core.Float64#740", 8
.set ".L+Core.Float64#740.jit", 139704953807808
.size ".L+Core.Float64#740.jit", 8
.section ".note.GNU-stack","",@progbits On this PR: julia> code_native(+, NTuple{2,Float64}; debuginfo=:none) .text
.file "+"
.section .ltext,"axl",@progbits
.globl "julia_+_682"
.p2align 4, 0x90
.type "julia_+_682",@function
"julia_+_682":
push rbp
mov rbp, rsp
vaddsd xmm0, xmm0, xmm1
pop rbp
ret
.Lfunc_end0:
.size "julia_+_682", .Lfunc_end0-"julia_+_682"
.type ".L+Core.Float64#684",@object
.section .lrodata,"al",@progbits
.p2align 3, 0x0
".L+Core.Float64#684":
.quad ".L+Core.Float64#684.jit"
.size ".L+Core.Float64#684", 8
.set ".L+Core.Float64#684.jit", 140587410418304
.size ".L+Core.Float64#684.jit", 8
.section ".note.GNU-stack","",@progbits Note that all the comments are gone. This makes reading complex code harder. I don't know if something changed in LLVM API to ask for the comments. |
Another issue is that SVE vectorisation on aarch64 seems to be broken (in the sense that it isn't used). Consider for example the function function axpy!(a, x, y)
@simd for idx in eachindex(x, y)
@inbounds y[idx] = muladd(a, x[idx], y[idx])
end
end With |
IIUC, someone deleted support for printing comments on our end:
This is now supposed to be set here rather than deleted: |
That is odd. The C++ standard says that this implicit destructor is not permitted to be called, so it is strange to see the analyzer thinking otherwise than what the standard permits. |
MWE for the apparent clang-analyzer mistake: vtjnash@deepsea4:~/julia$ clang++ -fPIE -std=c++20 dtor-test.cpp && ./a.out
C Ctor
C Dtor
vtjnash@deepsea4:~/julia$ g++ -fPIE -std=c++20 dtor-test.cpp && ./a.out
C Ctor
C Dtor
$ cat dtor-test.cpp
#include <iostream>
#include <optional>
class C {
int *once;
public:
C(int) {
std::cout << "C Ctor" << std::endl;
once = new int[3];
}
C(C&&) = delete;
~C() {
std::cout << "C Dtor" << std::endl;
delete[] once;
}
};
int main() {
std::optional<C> S{1};
return 0;
}
$ ./usr/tools/clang++ --analyze -analyzer-output=text -std=c++20 dtor-test.cpp
dtor-test.cpp:14:5: warning: Attempt to free released memory [cplusplus.NewDelete]
14 | delete[] once;
| ^ |
patch to add to fix this #56130 (comment) diff --git a/src/disasm.cpp b/src/disasm.cpp
index b944e48430c..cc296618635 100644
--- a/src/disasm.cpp
+++ b/src/disasm.cpp
@@ -873,6 +873,8 @@ static void jl_dump_asm_internal(
SourceMgr SrcMgr;
MCTargetOptions Options;
+ Options.AsmVerbose = true;
+ Options.MCUseDwarfDirectory = MCTargetOptions::EnableDwarfDirectory;
std::unique_ptr<MCAsmInfo> MAI(
TheTarget->createMCAsmInfo(*TheTarget->createMCRegInfo(TheTriple.str()), TheTriple.str(), Options));
assert(MAI && "Unable to create target asm info!");
@@ -1274,8 +1276,13 @@ jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char emit_mc, const
OutputAsmDialect = 1;
MCInstPrinter *InstPrinter = TM->getTarget().createMCInstPrinter(
jl_ExecutionEngine->getTargetTriple(), OutputAsmDialect, MAI, MII, MRI);
- std::unique_ptr<MCAsmBackend> MAB(TM->getTarget().createMCAsmBackend(
- STI, MRI, TM->Options.MCOptions));
+ MCTargetOptions Options = TM->Options.MCOptions;
+ Options.AsmVerbose = true;
+ Options.MCUseDwarfDirectory = MCTargetOptions::EnableDwarfDirectory;
+ if (binary)
+ Options.ShowMCEncoding = true;
+ std::unique_ptr<MCAsmBackend> MAB(TM->getTarget().createMCAsmBackend(
+ STI, MRI, Options));
std::unique_ptr<MCCodeEmitter> MCE;
if (binary) { // enable MCAsmStreamer::AddEncodingComment printing
MCE.reset(TM->getTarget().createMCCodeEmitter(MII, *Context));
@@ -1289,8 +1296,7 @@ jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char emit_mc, const
std::move(MAB), false
#endif
));
- std::unique_ptr<AsmPrinter> Printer(
- TM->getTarget().createAsmPrinter(*TM, std::move(S)));
+ AsmPrinter *Printer = TM->getTarget().createAsmPrinter(*TM, std::move(S));
#if JL_LLVM_VERSION >= 190000
Printer->addDebugHandler(
std::make_unique<LineNumberPrinterHandler>(*Printer, debuginfo));
@@ -1301,7 +1307,7 @@ jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char emit_mc, const
#endif
if (!Printer)
return jl_an_empty_string;
- PM.add(Printer.release());
+ PM.add(Printer);
PM.add(createFreeMachineFunctionPass());
TSM->withModuleDo([&](Module &m){ PM.run(m); });
}
The diff --git a/src/disasm.cpp b/src/disasm.cpp
index c3488afc4d9..40f23e869da 100644
--- a/src/disasm.cpp
+++ b/src/disasm.cpp
@@ -460,6 +460,9 @@ static void jl_strip_llvm_debug(Module *m, bool all_meta, LineNumberAnnotatedWri
if (AAW)
AAW->addDebugLoc(&inst, inst.getDebugLoc());
inst.setDebugLoc(DebugLoc());
+#if JL_LLVM_VERSION >= 190000
+ inst.dropDbgRecords();
+#endif
}
if (deletelast) {
deletelast->eraseFromParent(); |
Thanks for the patch but for reasons I don't understand yet, the following doesn't seem to work as debug info is not printed @@ -1274,8 +1276,13 @@ jl_value_t *jl_dump_function_asm_impl(jl_llvmf_dump_t* dump, char emit_mc, const
OutputAsmDialect = 1;
MCInstPrinter *InstPrinter = TM->getTarget().createMCInstPrinter(
jl_ExecutionEngine->getTargetTriple(), OutputAsmDialect, MAI, MII, MRI);
- std::unique_ptr<MCAsmBackend> MAB(TM->getTarget().createMCAsmBackend(
- STI, MRI, TM->Options.MCOptions));
+ MCTargetOptions Options = TM->Options.MCOptions;
+ Options.AsmVerbose = true;
+ Options.MCUseDwarfDirectory = MCTargetOptions::EnableDwarfDirectory;
+ if (binary)
+ Options.ShowMCEncoding = true;
+ std::unique_ptr<MCAsmBackend> MAB(TM->getTarget().createMCAsmBackend(
+ STI, MRI, Options));
std::unique_ptr<MCCodeEmitter> MCE;
if (binary) { // enable MCAsmStreamer::AddEncodingComment printing
MCE.reset(TM->getTarget().createMCCodeEmitter(MII, *Context)); Instead if I do the following, the debug info is printed correctly diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp
index e5a96cdf9d..a1fea54a16 100644
--- a/src/jitlayers.cpp
+++ b/src/jitlayers.cpp
@@ -1381,6 +1381,8 @@ namespace {
options.MCOptions.ABIName = "lp64";
#endif
#endif
+ options.MCOptions.AsmVerbose = true;
+ options.MCOptions.MCUseDwarfDirectory = MCTargetOptions::EnableDwarfDirectory;
uint32_t target_flags = 0;
auto target = jl_get_llvm_target(imaging_default(), target_flags);
auto &TheCPU = target.first; Also, the |
If that helps, the signature was added with #50630, to give an idea of where to look at in the code. |
Thanks. On the function signature not printing it looks like we null out the printer here which prevents the printing |
Including #55650 till that's merged.