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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

inference: remove the toplevel @nospecialize annotation from tfuncs.jl #47850

Merged
merged 1 commit into from
Dec 10, 2022

Conversation

aviatesk
Copy link
Sponsor Member

@aviatesk aviatesk commented Dec 9, 2022

Currently tfuncs.jl is full of @nospecialize annotations, and it is a bit confusing that we have both arg-wise annotation and toplevel annotation.

This commit switches to arg-wise annotation only, and also, omits most annotation with a new internal macro @nospecs:

@nospecs def

Adds @nospecialize annotation to non-annotated arguments of def.

(Core.Compiler) julia> @macroexpand @nospecs function tfunc(饾晝::AbstractLattice, x, y::Bool, zs...)
                       x, ys
                   end
:(function tfunc(\$(Expr(:meta, :specialize, :(饾晝::AbstractLattice))), x, y::Bool, zs...)
  #= REPL[3]:1 =#
  \$(Expr(:meta, :nospecialize, :x, :zs))
  #= REPL[3]:2 =#
  (x, ys)
end)

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

Base automatically changed from avi/lattice-tfuncs to master December 9, 2022 21:38
@aviatesk
Copy link
Sponsor Member Author

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@aviatesk
Copy link
Sponsor Member Author

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@aviatesk
Copy link
Sponsor Member Author

@Keno @vtjnash are you fine with this change?

@aviatesk
Copy link
Sponsor Member Author

@nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

@Keno
Copy link
Member

Keno commented Dec 10, 2022

I'm good with it - it always confuses me that the default in this file is inverted.

@aviatesk
Copy link
Sponsor Member Author

Ok, let's go with this.

@aviatesk aviatesk merged commit c41b44d into master Dec 10, 2022
@aviatesk aviatesk deleted the avi/tfuncs-specialize branch December 10, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants