JuliaSIMD / LoopVectorization.jl

Macro(s) for vectorizing loops.
MIT License
742 stars 66 forks source link

Release v0.12.167 breaks RecursiveFactorization on Julia v1.11+ #520

Closed staticfloat closed 8 months ago

staticfloat commented 9 months ago

If you try to precompile RecursiveFactorization against LoopVectorization v0.12.167+, you will receive the following error:

ERROR: LoadError: UndefVarError: `register_size` not defined in `LoopVectorization`                                                      
Stacktrace:                                                         
  [1] getproperty                                                                                                                        
    @ Base ./Base.jl:42 [inlined]
  [2] pick_threshold()
    @ RecursiveFactorization ~/.julia/packages/RecursiveFactorization/cDP6H/src/lu.jl:82
  [3] lu!
    @ RecursiveFactorization ~/.julia/packages/RecursiveFactorization/cDP6H/src/lu.jl:89 [inlined]

X-ref: https://github.com/JuliaLinearAlgebra/RecursiveFactorization.jl/blob/987606ff6e15d8a4f9645481c7a540d252c674da/src/lu.jl#L82

staticfloat commented 9 months ago

I argue that the disabling of Julia v1.11 should be done in a major release, to make it easier for others to set compat bounds due to changes like this.

I have opened a PR to General to yank the last two releases, to hopefully minimize the breakage: https://github.com/JuliaRegistries/General/pull/98171

chriselrod commented 9 months ago

I already made this PR yesterday: https://github.com/JuliaLinearAlgebra/RecursiveFactorization.jl/pull/89

I have opened a PR to General to yank the last two releases, to hopefully minimize the breakage:

I confess I didn't look into it too much, but was told LV was already broken on 1.11. But as that breakage was related to using Expr(:new, ...), I guess it may only happen in certain situations; that code path won't always be hit.

staticfloat commented 9 months ago

Fixing RecursiveFactorization is good, but this is a breaking change and should be done in a major version bump so that compat bounds are easier to set. As an example, if I have a compat bound that forces an older version of RecursiveFactorization, I can still get this newer version of LV quite easily. It would be better to release this as 0.13 or something, then go and change all older versions of RF to only be compatible with the 0.12 series.

staticfloat commented 9 months ago

I confess I didn't look into it too much, but was told LV was already broken on 1.11.

Perhaps there is something about it that is broken, but I can say that as of right now, if you run Pkg.add("RecursiveFactorization") on v1.11+, it fails to precompile because of this.

chriselrod commented 9 months ago

You can merge the yank. I tried a minimal example of Expr(:new, ...) locally, and it worked fine, but some form of this is now invalid.

I don't know the details, but I also don't think we should really bother maintaining this library.

maleadt commented 9 months ago

I also don't think we should really bother maintaining this library

Maybe we should then set compat bounds to prevent installation of LoopVectorization.jl on 1.11 so that downstream users are inclined to get rid of the LV.jl dependency? This issue currently causes multiple crashes on PkgEval, making it much harder to figure out when things regress. The alternative is blacklisting packages that use LoopVectorization.jl, which we don't want either because it means reducing coverage of PkgEval.

maleadt commented 9 months ago

Alternatively, can we not keep all of LoopVectorization.jl but just nuke the @turbo macro? That should keep RecursiveFactorization etc functional, without triggering the bad code generation that is incompatible with 1.11.

maleadt commented 9 months ago

https://github.com/JuliaSIMD/LoopVectorization.jl/pull/523

ranocha commented 9 months ago

I also don't think we should really bother maintaining this library

Maybe we should then set compat bounds to prevent installation of LoopVectorization.jl on 1.11 so that downstream users are inclined to get rid of the LV.jl dependency?

Is there an alternative that provides the same speedups as LoopVectorization.jl?

chriselrod commented 9 months ago

Is there an alternative that provides the same speedups as LoopVectorization.jl?

No.

Alternatively, can we not keep all of LoopVectorization.jl but just nuke the @turbo macro? That should keep RecursiveFactorization etc functional, without triggering the bad code generation that is incompatible with 1.11.

Would be nice to avoid the lengthy compile times, as it is part of a long dependency chain that reduces the benefit of parallel precompilation, but that's fine.

maleadt commented 9 months ago

Is there an alternative that provides the same speedups as LoopVectorization.jl?

No.

Hopefully we can just keep LoopVectorization.jl with https://github.com/JuliaSIMD/LoopVectorization.jl/pull/524.

maleadt commented 8 months ago

The release has been yanked, so I guess this can be closed?