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.

Sign up for GitHub

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

Jump to bottom

type inference regression in 1.10.0-alpha1 when using @propagate_inbounds #50544

Closed
matthias314 opened this issue Jul 13, 2023 · 1 comment · Fixed by #50561
Closed

type inference regression in 1.10.0-alpha1 when using @propagate_inbounds #50544

matthias314 opened this issue Jul 13, 2023 · 1 comment · Fixed by #50561

Comments

@matthias314
Copy link
Contributor

I've noticed this in real code, but here is a MWE:

struct A{T<:Tuple} t::T; end

Base.@propagate_inbounds f(x, i, ii...) = f(f(x, i), ii...)   
Base.@propagate_inbounds f(::Type{A{T}}, i) where T = T.parameters[i]

g(T...) = A{Tuple{T...}}

h(x::T) where T = g(f(T, 1), f(T, 2, 1))

a = A((1, A((2.0, 'x'))))
@code_typed h(a)

With 1.10.0-alpha I get

=> Type{A{var"#s173"}} where var"#s173"<:Tuple{Int64, Vararg}

With 1.9.2 it's

=> Type{A{Tuple{Int64, Float64}}}

Without @propagate_inbounds the inference works also for 1.10.0-alpha1.

My understanding is that Julia's type inference capabilities are not guaranteed. So maybe you think this is a bug, maybe not.

Julia Version 1.10.0-alpha1
Commit f8ad15f7b16 (2023-07-06 10:36 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 24 × Intel(R) Xeon(R) CPU E5-2430 0 @ 2.20GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, sandybridge)
  Threads: 1 on 24 virtual cores
@maleadt
Copy link
Member

maleadt commented Jul 14, 2023

Bisected to #48684 (cc @aviatesk).

aviatesk added a commit that referenced this issue Jul 15, 2023
@aviatesk
remove :boundscheck argument from Core._svec_ref
cd25161 
`Core._svec_ref` has accepted `boundscheck`-value as the first argument
since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls
`jl_svec_ref` in either the interpreter or the codegen, and thus the
`boundscheck` value isn't utilized in any optimizations. Rather, even
worse, this `boundscheck`-argument negatively influences the effect
analysis (xref #50167 for details) and has caused type inference
regressions as reported in #50544.

For these reasons, this commit simply eliminates the `boundscheck`
argument from `Core._svec_ref`. Consequently,
`getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible.

closes #50544
@aviatesk aviatesk mentioned this issue Jul 15, 2023
Merged
aviatesk added a commit that referenced this issue Jul 15, 2023
@aviatesk
remove :boundscheck argument from Core._svec_ref
3cc76a3 
`Core._svec_ref` has accepted `boundscheck`-value as the first argument
since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls
`jl_svec_ref` in either the interpreter or the codegen, and thus the
`boundscheck` value isn't utilized in any optimizations. Rather, even
worse, this `boundscheck`-argument negatively influences the effect
analysis (xref #50167 for details) and has caused type inference
regressions as reported in #50544.

For these reasons, this commit simply eliminates the `boundscheck`
argument from `Core._svec_ref`. Consequently,
`getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible.

closes #50544
aviatesk added a commit that referenced this issue Jul 15, 2023
@aviatesk
remove :boundscheck argument from Core._svec_ref
94ad38e 
`Core._svec_ref` has accepted `boundscheck`-value as the first argument
since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls
`jl_svec_ref` in either the interpreter or the codegen, and thus the
`boundscheck` value isn't utilized in any optimizations. Rather, even
worse, this `boundscheck`-argument negatively influences the effect
analysis (xref #50167 for details) and has caused type inference
regressions as reported in #50544.

For these reasons, this commit simply eliminates the `boundscheck`
argument from `Core._svec_ref`. Consequently,
`getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible.

closes #50544
aviatesk added a commit that referenced this issue Jul 17, 2023
@aviatesk
remove :boundscheck argument from Core._svec_ref (#50561)
1964621 
`Core._svec_ref` has accepted `boundscheck`-value as the first argument
since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls
`jl_svec_ref` in either the interpreter or the codegen, and thus the
`boundscheck` value isn't utilized in any optimizations. Rather, even
worse, this `boundscheck`-argument negatively influences the effect
analysis (xref #50167 for details) and has caused type inference
regressions as reported in #50544.

For these reasons, this commit simply eliminates the `boundscheck`
argument from `Core._svec_ref`. Consequently,
`getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible.

closes #50544
aviatesk added a commit that referenced this issue Jul 17, 2023
@aviatesk
remove :boundscheck argument from Core._svec_ref (#50561)
2753ef9 
`Core._svec_ref` has accepted `boundscheck`-value as the first argument
since it was added in #45062. Nonetheless, `Core._svec_ref` simply calls
`jl_svec_ref` in either the interpreter or the codegen, and thus the
`boundscheck` value isn't utilized in any optimizations. Rather, even
worse, this `boundscheck`-argument negatively influences the effect
analysis (xref #50167 for details) and has caused type inference
regressions as reported in #50544.

For these reasons, this commit simply eliminates the `boundscheck`
argument from `Core._svec_ref`. Consequently,
`getindex(::SimpleVector, ::Int)` is now being concrete-eval eligible.

closes #50544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

remove :boundscheck argument from Core._svec_ref JuliaLang/julia
2 participants
@maleadt @matthias314