Skip to content

Commit

Permalink
sroa: Don't use unwrapped type for type constraint (JuliaLang#50499)
Browse files Browse the repository at this point in the history
In SROA, when we lift getfields over branches (ifelse or phi), we
try to exclude branches that we know to not contribute by their
types. However, we were incorrectly using the `unwrap_unionall`'ed
version of the type. Type intersection has a bunch of fallbacks for
free typevars, but the results are not necessarily correct (e.g.
in the test case where `hasintersect(Wrap1{Wrap{Int}}, Wrap1{Wrap{T}})`
gives false). We should ideally get around to just making type-quries
for things with free typevars an error, but for now, just fix the
particular issue in sroa, by using the non-unwrapped type.
  • Loading branch information
Keno committed Jul 11, 2023
1 parent b26f3b2 commit 3a36e1a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
16 changes: 8 additions & 8 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1105,25 +1105,25 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
# analyze `getfield` / `isdefined` / `setfield!` call
val = stmt.args[2]
end
struct_typ = unwrap_unionall(widenconst(argextype(val, compact)))
struct_typ = widenconst(argextype(val, compact))
struct_typ_unwrapped = unwrap_unionall(struct_typ)
if isa(struct_typ, Union) && struct_typ <: Tuple
struct_typ = unswitchtupleunion(struct_typ)
struct_typ_unwrapped = unswitchtupleunion(struct_typ_unwrapped)
end
if isa(struct_typ, Union) && is_isdefined
if isa(struct_typ_unwrapped, Union) && is_isdefined
lift_comparison!(isdefined, compact, idx, stmt, lifting_cache, 𝕃ₒ)
continue
end
isa(struct_typ, DataType) || continue
isa(struct_typ_unwrapped, DataType) || continue

struct_typ.name.atomicfields == C_NULL || continue # TODO: handle more
struct_typ_unwrapped.name.atomicfields == C_NULL || continue # TODO: handle more
if !((field_ordering === :unspecified) ||
(field_ordering isa Const && field_ordering.val === :not_atomic))
continue
end


# analyze this mutable struct here for the later pass
if ismutabletype(struct_typ)
if ismutabletype(struct_typ_unwrapped)
isa(val, SSAValue) || continue
let intermediaries = SPCSet()
callback = IntermediaryCollector(intermediaries)
Expand Down Expand Up @@ -1153,7 +1153,7 @@ function sroa_pass!(ir::IRCode, inlining::Union{Nothing,InliningState}=nothing)
end

# perform SROA on immutable structs here on
field = try_compute_fieldidx_stmt(compact, stmt, struct_typ)
field = try_compute_fieldidx_stmt(compact, stmt, struct_typ_unwrapped)
field === nothing && continue

leaves, visited_philikes = collect_leaves(compact, val, struct_typ, 𝕃ₒ, phi_or_ifelse_predecessors)
Expand Down
19 changes: 19 additions & 0 deletions test/compiler/irpasses.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1371,3 +1371,22 @@ struct TParamTypeofTest2{S,T}
end
tparam_typeof_test_elim2(x, y) = TParamTypeofTest2(x, y).x
@test fully_eliminated(tparam_typeof_test_elim2, Tuple{Any,Any})

# Test that sroa doesn't get confused by free type parameters in struct types
struct Wrap1{T}
x::T
@eval @inline (T::Type{Wrap1{X}} where X)(x) = $(Expr(:new, :T, :x))
end
Wrap1(x) = Wrap1{typeof(x)}(x)

function wrap1_wrap1_ifelse(b, x, w1)
w2 = Wrap1(Wrap1(x))
w3 = Wrap1(typeof(w1)(w1.x))
Core.ifelse(b, w3, w2).x.x
end
function wrap1_wrap1_wrapper(b, x, y)
w1 = Base.inferencebarrier(Wrap1(y))::Wrap1{<:Union{Int, Float64}}
wrap1_wrap1_ifelse(b, x, w1)
end
@test wrap1_wrap1_wrapper(true, 1, 1.0) === 1.0
@test wrap1_wrap1_wrapper(false, 1, 1.0) === 1

0 comments on commit 3a36e1a

Please sign in to comment.