Skip to content
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

Skip libraries consisting of the empty string #50899

Merged
merged 4 commits into from
Aug 15, 2023
Merged

Conversation

pchintalapudi
Copy link
Member

Should fix #50745 (comment) and #6184 (comment), probably.

@pchintalapudi pchintalapudi added compiler:codegen Generation of LLVM IR and native code bugfix This change fixes an existing bug compiler:llvm For issues that relate to LLVM labels Aug 13, 2023
@gbaraldi
Copy link
Member

LGTM

Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really familiar with the pass, but SGTM.

@pchintalapudi
Copy link
Member Author

The test only fails due to #22318.

test/ccall.jl Outdated Show resolved Hide resolved
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@maleadt maleadt added the merge me PR is reviewed. Merge when all tests are passing label Aug 15, 2023
@maleadt
Copy link
Member

maleadt commented Aug 15, 2023

CI looks good, apart from macOS x86_64 which is backlogged. I'm going to go ahead and merge this already, as it's unlikely that this would cause an issue on only that platform, and without it there's many crashes on PkgEval.

@maleadt maleadt merged commit 0b190b3 into master Aug 15, 2023
2 checks passed
@maleadt maleadt deleted the pc/skip-uninit-library branch August 15, 2023 15:07
@maleadt
Copy link
Member

maleadt commented Aug 16, 2023

This has not fixed the PkgEval issue I mentioned, see https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_date/2023-08/15/report.html

For example:

Failed to precompile SymPy [24249f21-da20-56a4-8eb1-6a02cf4ae2e6] to "/home/pkgeval/.julia/compiled/v1.11/SymPy/jl_wEH7oC".
julia: /source/usr/include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From*) [with To = llvm::ConstantExpr; From = llvm::Value]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

[496] signal (6.-6): Aborted
in expression starting at /home/pkgeval/.julia/packages/SymPy/AswGW/src/SymPy.jl:39
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7f33aa10c40e)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
cast<llvm::ConstantExpr, llvm::Value> at /source/usr/include/llvm/Support/Casting.h:578 [inlined]
operator() at /source/src/jitlayers.cpp:1597
optimizeDLSyms at /source/src/jitlayers.cpp:2032 [inlined]
_jl_compile_codeinst at /source/src/jitlayers.cpp:227
jl_generate_fptr_impl at /source/src/jitlayers.cpp:525
jl_compile_method_internal at /source/src/gf.c:2475 [inlined]
jl_compile_method_internal at /source/src/gf.c:2363
gen_cfun_wrapper at /source/src/codegen.cpp:6079
emit_cfunction at /source/src/codegen.cpp:6716 [inlined]
emit_expr at /source/src/codegen.cpp:5547
emit_ssaval_assign at /source/src/codegen.cpp:5119
emit_stmtpos at /source/src/codegen.cpp:5354 [inlined]
emit_function at /source/src/codegen.cpp:8467

@gbaraldi
Copy link
Member

I can't reproduce this failure, does pkgeval have this PR?

@maleadt
Copy link
Member

maleadt commented Aug 16, 2023

Yes. You can check the versioninfo() that's part of every log, e.g.: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_date/2023-08/15/Roots.primary.log, which shows 0b190b3, the very commit where this Pr was merged.

@vtjnash
Copy link
Member

vtjnash commented Aug 17, 2023

doesn't seem to have fixed this case either:

julia> ccall(:test, Int, ())
julia: /home/vtjnash/julia1/src/jitlayers.cpp:1597: void JuliaOJIT::DLSymOptimizer::operator()(llvm::Module&): Assertion `cast<ConstantExpr>(libarg)->getOpcode() == Instruction::IntToPtr && "libarg should be either a global variable or a integer index!"' failed.

[2617529] signal (6.-6): Aborted
in expression starting at REPL[1]:1
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7f3323207728)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
operator() at /home/vtjnash/julia1/src/jitlayers.cpp:1597
optimizeDLSyms at /home/vtjnash/julia1/src/jitlayers.cpp:2032 [inlined]

julia> code_llvm(()) do; ccall(:test, Int, ()); end
define i64 @"julia_#3_496"() #1 {
top:
  %test1 = load atomic i64 ()*, i64 ()** bitcast (void ()** @jlplt_test_499_got to i64 ()**) unordered, align 8
  %0 = call i64 %test1()
  ret i64 %0
}

@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug compiler:codegen Generation of LLVM IR and native code compiler:llvm For issues that relate to LLVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants