Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This allows the julia-level optimizer to drop unused pointerref. This matches LLVM in that unused non-volatile loads are allowed to be dropped, despite them having possibly observable side effects (segfaults), since those are modeled as undefined. When heavily optimizing code we frequently saw a lot of left over pointerref calls resulting from inlining `length(::SimpleVector)`. We have a special tfunc for that call (that can give us the result as a const), but inline it anyway and thus end up with a pointerref whose result is unused that we didn't used to be able to eliminate. E.g. ``` julia> f(T::Type{S}) where {S} = Val(length(T.parameters)) f (generic function with 1 method) ``` Before: ``` julia> @code_typed f(Int) CodeInfo( 1 ─ %1 = (Base.getfield)(T, :parameters)::SimpleVector │ %2 = $(Expr(:gc_preserve_begin, :(%1))) │ %3 = $(Expr(:foreigncall, :(:jl_value_ptr), Ptr{Nothing}, svec(Any), :(:ccall), 1, :(%1)))::Ptr{Nothing} │ %4 = (Base.bitcast)(Ptr{Int64}, %3)::Ptr{Int64} │ (Base.pointerref)(%4, 1, 1)::Int64 │ $(Expr(:gc_preserve_end, :(%2))) └── return $(QuoteNode(Val{0}())) ) => Val{0} ``` After: ``` julia> @code_typed f(Int) CodeInfo( 1 ─ %1 = (Base.getfield)(T, :parameters)::SimpleVector │ %2 = $(Expr(:gc_preserve_begin, :(%1))) │ $(Expr(:foreigncall, :(:jl_value_ptr), Ptr{Nothing}, svec(Any), :(:ccall), 1, :(%1)))::Ptr{Nothing} │ $(Expr(:gc_preserve_end, :(%2))) └── return $(QuoteNode(Val{0}())) ) => Val{0} ``` Of course we still have the useless foreigncall. That is outside the scope of this PR, but I'm hoping to have a general solution in the future to mark foreigncalls as effect free if unused.
- Loading branch information
236c69b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executing the daily benchmark build, I will reply here when finished:
@nanosoldier
runbenchmarks(ALL, isdaily = true)
236c69b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan