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.

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

lattice: Allow external lattice to have a say in how to widen #47307

Merged
merged 1 commit into from
Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions base/compiler/abstractlattice.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ extensions.
"""
struct JLTypeLattice <: AbstractLattice; end
widenlattice(::JLTypeLattice) = error("Type lattice is the least-precise lattice available")
is_valid_lattice(::JLTypeLattice, @nospecialize(elem)) = isa(elem, Type)
is_valid_lattice(lattice::JLTypeLattice, @nospecialize(elem)) = is_valid_lattice_norec(lattice, elem)
is_valid_lattice_norec(::JLTypeLattice, @nospecialize(elem)) = isa(elem, Type)

"""
struct ConstsLattice
Expand All @@ -18,8 +19,7 @@ A lattice extending `JLTypeLattice` and adjoining `Const` and `PartialTypeVar`.
"""
struct ConstsLattice <: AbstractLattice; end
widenlattice(::ConstsLattice) = JLTypeLattice()
is_valid_lattice(lattice::ConstsLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) || isa(elem, Const) || isa(elem, PartialTypeVar)
is_valid_lattice_norec(lattice::ConstsLattice, @nospecialize(elem)) = isa(elem, Const) || isa(elem, PartialTypeVar)

"""
struct PartialsLattice{L}
Expand All @@ -30,9 +30,7 @@ struct PartialsLattice{L <: AbstractLattice} <: AbstractLattice
parent::L
end
widenlattice(L::PartialsLattice) = L.parent
is_valid_lattice(lattice::PartialsLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) ||
isa(elem, PartialStruct) || isa(elem, PartialOpaque)
is_valid_lattice_norec(lattice::PartialsLattice, @nospecialize(elem)) = isa(elem, PartialStruct) || isa(elem, PartialOpaque)

"""
struct ConditionalsLattice{L}
Expand All @@ -43,15 +41,13 @@ struct ConditionalsLattice{L <: AbstractLattice} <: AbstractLattice
parent::L
end
widenlattice(L::ConditionalsLattice) = L.parent
is_valid_lattice(lattice::ConditionalsLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) || isa(elem, Conditional)
is_valid_lattice_norec(lattice::ConditionalsLattice, @nospecialize(elem)) = isa(elem, Conditional)

struct InterConditionalsLattice{L <: AbstractLattice} <: AbstractLattice
parent::L
end
widenlattice(L::InterConditionalsLattice) = L.parent
is_valid_lattice(lattice::InterConditionalsLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) || isa(elem, InterConditional)
is_valid_lattice_norec(lattice::InterConditionalsLattice, @nospecialize(elem)) = isa(elem, InterConditional)

const AnyConditionalsLattice{L} = Union{ConditionalsLattice{L}, InterConditionalsLattice{L}}
const BaseInferenceLattice = typeof(ConditionalsLattice(PartialsLattice(ConstsLattice())))
Expand All @@ -67,8 +63,7 @@ struct InferenceLattice{L} <: AbstractLattice
parent::L
end
widenlattice(L::InferenceLattice) = L.parent
is_valid_lattice(lattice::InferenceLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) || isa(elem, LimitedAccuracy)
is_valid_lattice_norec(lattice::InferenceLattice, @nospecialize(elem)) = isa(elem, LimitedAccuracy)

"""
struct OptimizerLattice
Expand All @@ -81,8 +76,7 @@ struct OptimizerLattice{L} <: AbstractLattice
end
OptimizerLattice() = OptimizerLattice(BaseInferenceLattice.instance)
widenlattice(L::OptimizerLattice) = L.parent
is_valid_lattice(lattice::OptimizerLattice, @nospecialize(elem)) =
is_valid_lattice(widenlattice(lattice), elem) || isa(elem, MaybeUndef)
is_valid_lattice_norec(lattice::OptimizerLattice, @nospecialize(elem)) = isa(elem, MaybeUndef)

"""
tmeet(lattice, a, b::Type)
Expand Down Expand Up @@ -174,3 +168,13 @@ tmerge(@nospecialize(a), @nospecialize(b)) = tmerge(fallback_lattice, a, b)
(@nospecialize(a), @nospecialize(b)) = (fallback_lattice, a, b)
(@nospecialize(a), @nospecialize(b)) = (fallback_lattice, a, b)
is_lattice_equal(@nospecialize(a), @nospecialize(b)) = is_lattice_equal(fallback_lattice, a, b)

is_valid_lattice(lattice::AbstractLattice, @nospecialize(elem)) = is_valid_lattice_norec(lattice, elem) &&
is_valid_lattice(widenlattice(lattice), elem)

# Widenlattice with argument
widenlattice(::JLTypeLattice, @nospecialize(t)) = widenconst(t)
function widenlattice(lattice::AbstractLattice, @nospecialize(t))
is_valid_lattice_norec(lattice, t) && return t
widenlattice(widenlattice(lattice), t)
end
22 changes: 15 additions & 7 deletions base/compiler/typelimits.jl
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ function tmerge(lattice::InterConditionalsLattice, @nospecialize(typea), @nospec
end
return Bool
end
typea = widenconditional(typea)
typeb = widenconditional(typeb)
return tmerge(widenlattice(lattice), typea, typeb)
Comment on lines +471 to 473
Copy link
Sponsor Member

@vtjnash vtjnash Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no lattice? (overall, SGTM, though I only partially follow the intended need here)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean why spell this as widenconditional rather than widenlattice? The lack of widenconditional here was just an oversight in the original PR. It's there in the non-IPO version of this function. It was exposed here, because I stopped doing an unconditional widenconst at the Consts layer. I do think this could be spelled as widenlattice also, but I wanted to minimize the impact for the time being, since I don't actually have an example of a lattice layer inserted at a place where it would make a difference.

end

Expand Down Expand Up @@ -524,10 +526,13 @@ function tmerge(lattice::PartialsLattice, @nospecialize(typea), @nospecialize(ty
return anyrefine ? PartialStruct(aty, fields) : aty
end
end


# Don't widen const here - external AbstractInterpreter might insert lattice
# layers between us and `ConstsLattice`.
isa(typea, PartialStruct) && (typea = widenconst(typea))
isa(typeb, PartialStruct) && (typeb = widenconst(typeb))
wl = widenlattice(lattice)
isa(typea, PartialStruct) && (typea = widenlattice(wl, typea))
isa(typeb, PartialStruct) && (typeb = widenlattice(wl, typeb))

# type-lattice for PartialOpaque wrapper
apo = isa(typea, PartialOpaque)
Expand All @@ -540,24 +545,27 @@ function tmerge(lattice::PartialsLattice, @nospecialize(typea), @nospecialize(ty
typea.parent === typeb.parent)
return widenconst(typea)
end
return PartialOpaque(typea.typ, tmerge(typea.env, typeb.env),
return PartialOpaque(typea.typ, tmerge(lattice, typea.env, typeb.env),
typea.parent, typea.source)
end
typea = aty
typeb = bty
elseif apo
typea = widenconst(typea)
typea = widenlattice(wl, typea)
elseif bpo
typeb = widenconst(typeb)
typeb = widenlattice(wl, typeb)
end

return tmerge(widenlattice(lattice), typea, typeb)
return tmerge(wl, typea, typeb)
end

function tmerge(lattice::ConstsLattice, @nospecialize(typea), @nospecialize(typeb))
# the equality of the constants can be checked here, but the equivalent check is usually
# done by `tmerge_fast_path` at earlier lattice stage
return tmerge(widenlattice(lattice), widenconst(typea), widenconst(typeb))
wl = widenlattice(lattice)
(isa(typea, Const) || isa(typea, PartialTypeVar)) && (typea = widenlattice(wl, typea))
(isa(typeb, Const) || isa(typeb, PartialTypeVar)) && (typeb = widenlattice(wl, typeb))
return tmerge(wl, typea, typeb)
end

function tmerge(::JLTypeLattice, @nospecialize(typea::Type), @nospecialize(typeb::Type))
Expand Down
2 changes: 1 addition & 1 deletion test/compiler/AbstractInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ function CC.tmerge(𝕃::AnyTaintLattice, @nospecialize(typea), @nospecialize(ty
if isa(typea, T)
if isa(typeb, T)
return T(
tmerge(widenlattice(𝕃), typea.typ, typeb),
tmerge(widenlattice(𝕃), typea.typ, typeb.typ),
typea.slots ∪ typeb.slots)
else
typea = typea.typ
Expand Down