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

Type-stable codegen for specialized in(v, ::Tuple) #54026

Merged
merged 3 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
fix up and avoid inference blowup
  • Loading branch information
aviatesk committed May 9, 2024
commit 703de63227980eff57e4e8af3bc20a3e7340a88f
13 changes: 7 additions & 6 deletions base/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1347,13 +1347,11 @@ end

# Specialized variant of in for Tuple, which can generate typed comparisons for each element
# of the tuple, skipping values that are statically known to be != at compile time.
function in(x, itr::Tuple)
@_terminates_globally
_in_tuple(x, itr, false)
end
in(x, itr::Tuple) = _in_tuple(x, itr, false)
# This recursive function will be unrolled at compiletime, and will not generate separate
# llvm-compiled specializations for each step of the recursion.
@inline function _in_tuple(x, @nospecialize(itr::Tuple), anymissing::Bool)
function _in_tuple(x, @nospecialize(itr::Tuple), anymissing::Bool)
@inline
Comment on lines +1353 to +1354
Copy link
Member Author

Choose a reason for hiding this comment

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

Can you help me understand why you moved the inline annotation? Does it have any difference, or you just prefer this stylistically? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. An @inline applied to a method carries less weight than an @inline applied to code block within the method body.

  2. The semantics are different, when used within the method body it suggests to inline callees.

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW can you help review my linked PR, given that you're freshly acquainted with the code?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

They have the same effect, but @inline method def is defined in base/expr.jl and is not available at this point of loading base/operators.jl when building system image. the version which is used without any arguments on the other hand is defined at the very early stage of sysimg build, so we can use it even before loading base/expr.jl.

# Base case
if isempty(itr)
return anymissing ? missing : false
Expand All @@ -1365,9 +1363,12 @@ end
elseif v
return true
end
return _in_tuple(x, Base.tail(itr), anymissing)
return _in_tuple(x, tail(itr), anymissing)
end

# fallback to the loop implementation after some number of arguments to avoid inference blowup
in(x, itr::Any32) = invoke(in, Tuple{Any,Any}, x, itr)

const ∈ = in
∉(x, itr) = !∈(x, itr)
∉(itr) = Fix2(∉, itr)
Expand Down
24 changes: 19 additions & 5 deletions test/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ end
@test B46327() <= B46327()
end

@testset "concrete eval `x in itr::Tuple`" begin
@testset "inference for `x in itr::Tuple`" begin
# concrete evaluation
@test Core.Compiler.is_foldable(Base.infer_effects(in, (Int,Tuple{Int,Int,Int})))
@test Core.Compiler.is_foldable(Base.infer_effects(in, (Char,Tuple{Char,Char,Char})))
for i = (1,2,3)
Expand All @@ -389,10 +390,23 @@ end
end |> only == Val{true}
end
end
@test Base.return_types() do
@test Base.infer_return_type() do
Val(4 in (1,2,3))
end |> only == Val{false}
@test Base.return_types() do
end == Val{false}
@test Base.infer_return_type() do
Val('1' in ('1','2','3'))
end |> only == Val{true}
end == Val{true}

# constant propagation
@test Base.infer_return_type((Int,Int)) do x, y
Val(1 in (x,2,y))
end >: Val{true}
@test Base.infer_return_type((Int,Int)) do x, y
Val(2 in (x,2,y))
end == Val{true}

# should use the loop implementation given large tuples to avoid inference blowup
let t = ntuple(x->'A', 10000);
@test Base.infer_return_type(in, (Char,typeof(t))) == Bool
end
end