-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
optimizer: improve general code quality #42357
Conversation
base/compiler/ssair/passes.jl
Outdated
@@ -110,7 +110,8 @@ function compute_value_for_use(ir::IRCode, domtree::DomTree, allblocks::Vector{I | |||
end | |||
end | |||
|
|||
function simple_walk(compact::IncrementalCompact, @nospecialize(defssa#=::AnySSAValue=#), pi_callback=(pi, idx)->false) | |||
function simple_walk(compact::IncrementalCompact, @nospecialize(defssa::AnySSAValue), |
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.
Declaring a type on a @nospecialize
argument is normally a bad idea, as it may severely penalize the call
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.
Would you mind elaborating it ?
@nospecialize
influences dispatches only at this moment, and I think every call of this function will be statically resolved anyway. And so I don't think there is any other effects than now we can get MethodError
for wrong defssa
types.
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.
it removes it from the dispatch cache
base/compiler/ssair/passes.jl
Outdated
@@ -148,12 +149,12 @@ function simple_walk(compact::IncrementalCompact, @nospecialize(defssa#=::AnySSA | |||
end | |||
end | |||
|
|||
function simple_walk_constraint(compact::IncrementalCompact, @nospecialize(defidx), @nospecialize(typeconstraint) = types(compact)[defidx]) | |||
function simple_walk_constraint(compact::IncrementalCompact, @nospecialize(defssa::AnySSAValue), @nospecialize(typeconstraint) = types(compact)[defssa]) |
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.
same here
# global assertion_counter | ||
# assertion_counter::Int += 1 |
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.
🙀
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.
We need another optimizer that optimizes our optimizer :p
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.
A bit seriously, we may want to put more efforts on integrating JET with Julia base.
a023da6
to
d936b81
Compare
Should be good to go once CI passes successfully. |
Built on top of #42355: - add more type signatures - add more `@nospecialize` decls - remove dead/debug code - add some docs on SROA and ADCE passes
- add more type signatures - add more `@nospecialize` decls - remove dead/debug code - add some docs on SROA and ADCE passes
- add more type signatures - add more `@nospecialize` decls - remove dead/debug code - add some docs on SROA and ADCE passes
Built on top of #42355:
@nospecialize
decls for callbacks