VectorizedReduction.jl icon indicating copy to clipboard operation
VectorizedReduction.jl copied to clipboard

Segfaults on Julia 1.11

Open KristofferC opened this issue 1 year ago • 6 comments
trafficstars

This package segfaults on upcoming Julia 1.11 (https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/0520b80_vs_997b49f/VectorizedReduction.primary.log) with

in expression starting at /home/pkgeval/.julia/packages/VectorizedReduction/bsnWJ/test/reduce.jl:4
macro expansion at /home/pkgeval/.julia/packages/VectorizationBase/xE5Tx/src/llvm_intrin/binary_ops.jl:31 [inlined]
vadd_fast at /home/pkgeval/.julia/packages/VectorizationBase/xE5Tx/src/llvm_intrin/binary_ops.jl:31 [inlined]
fmap at /home/pkgeval/.julia/packages/VectorizationBase/xE5Tx/src/vecunroll/fmap.jl:11 [inlined]
vadd_fast at /home/pkgeval/.julia/packages/VectorizationBase/xE5Tx/src/vecunroll/fmap.jl:111 [inlined]
add_fast at /home/pkgeval/.julia/packages/VectorizationBase/xE5Tx/src/base_defs.jl:91 [inlined]
macro expansion at /home/pkgeval/.julia/packages/LoopVectorization/7gWfp/src/reconstruct_loopset.jl:1107 [inlined]
_turbo_! at /home/pkgeval/.julia/packages/LoopVectorization/7gWfp/src/reconstruct_loopset.jl:1107 [inlined]
macro expansion at /home/pkgeval/.julia/packages/VectorizedReduction/bsnWJ/src/vmapreduce.jl:236 [inlined]
vvmapreduce at /home/pkgeval/.julia/packages/VectorizedReduction/bsnWJ/src/vmapreduce.jl:231
vvreduce at /home/pkgeval/.julia/packages/VectorizedReduction/bsnWJ/src/vmapreduce.jl:147
unknown function (ip: 0x7fdeccb92373)
_jl_invoke at /source/src/gf.c:2945 [inlined]
ijl_apply_generic at /source/src/gf.c:3122
jl_apply at /source/src/julia.h:2165 [inlined]
do_call at /source/src/interpreter.c:126

Note that the LoopVectorization + VectorizationBase ecosystem is deprecated on 1.11 (due to issues like this) so it might be best to avoid them for now

KristofferC avatar Mar 05 '24 20:03 KristofferC

No choice but to deprecate this package on 1.11, as it is little more than codegen piped to LoopVectorization. I'll update the README and semver constraints accordingly.

andrewjradcliffe avatar Mar 29 '24 23:03 andrewjradcliffe

I believe issue here is that when reducing over an empty collection, code like the following is generated:

A = Float64[]
out = zero(eltype(A))
@turbo for i in eachindex(A)
    out += A[i]
end

LoopVectorization warns users not iterate over an empty collection. Changing to @turbo check_empty=true for i in eachindex(A) should be a quick fix. Admittedly, this is only worthwhile if LoopVectorization does not get deprecated.

schrimpf avatar May 01 '24 23:05 schrimpf

Thanks for the debugging, Paul. That is indeed the general form of the cause of the segfaults on Julia 1.11. It was not much more effort than a find and replace (easy with projectile), so I went ahead and applied it.

LoopVectorization is the major source of performance, but the code generation for involving multiple multidimensional arrays is identical to the optimal loop structure one would write by hand, hence, it can still be quite convenient to use the functions. However, the interface leaves something to be desired (vv, vt prefixes are verbose and don't clearly map to Base versions). I plan to reassess interest once 1.11 is the stable, and if there is interest, create a clean interface. Interfaces aside, an obvious improvement is to switch to Base.Threads for the multi-threaded loops, since Polyester will also be deprecated.

andrewjradcliffe avatar May 07 '24 02:05 andrewjradcliffe

FWIW, all of LoopVectorization's tests pass on 1.11 beta, so I wouldn't bother transitioning off until you have a reason. https://github.com/JuliaSIMD/LoopVectorization.jl/actions/runs/8946228352

Maybe check_empty=true should be the default.

chriselrod avatar May 07 '24 11:05 chriselrod

Maybe check_empty=true should be the default.

It is now the default in the latest release of this package. I should have set it as the default 2 years ago -_-;;

all of LoopVectorization's tests pass on 1.11 beta

I noticed that. Furthermore, the test suite for this package (which forces LoopVectorization to compile a wide variety of code) passes on Julia master branch.

so I wouldn't bother transitioning off until you have a reason.

Completely agree.

andrewjradcliffe avatar May 08 '24 00:05 andrewjradcliffe

I noticed that. Furthermore, the test suite for this package (which forces LoopVectorization to compile a wide variety of code) passes on Julia master branch.

LoopVectorization's own test failures on master are from checking that nothing printed to stderr: https://github.com/JuliaSIMD/LoopVectorization.jl/actions/runs/8946228355/job/24576518019 We suddenly get a lot of warnings printing about https://github.com/JuliaLang/julia/pull/53687 I think it'd also be good to switch the llvmcalls that LV uses to opaque pointers at the same time, as opaque pointer mode becomes the default in LLVM 17 and will eventually be dropped.

chriselrod avatar May 08 '24 02:05 chriselrod