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

inference: Switch to typename-based override for constprop heuristics #55288

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

Keno
Copy link
Member

@Keno Keno commented Jul 28, 2024

As mentioned in #55271, I'd like to get rid of istopfunction for various reasons. Rather than hardcoding constprop overrides based on binding name, this annotates the relevant TypeNames (like the existing max_methods override), removing the binding lookups from inference.

As mentioned in #55271, I'd like to get rid of `istopfunction` for various reasons.
Rather than hardcoding constprop overrides based on binding name, this annotates
the relevant TypeNames (like the existing `max_methods` override), removing the
binding lookups from inference.
@Keno Keno requested review from vtjnash and aviatesk July 28, 2024 16:37
Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

This might be the best way to implement the special cases.
I came up with this alternative, but I don't think this is necessarily more elegant than this approach:

Core.Compiler

const functions_with_sametype_heuristic = Any[]

base/essentials.jl

for f = Any[+, -, *, ==, !=, <=, >=, <, >, <<, >>]
    push!(Core.Compiler.functions_with_sametype_heuristic, f)
end

@Keno
Copy link
Member Author

Keno commented Jul 29, 2024

The problem with using an array in the compiler is that you can't use it in pkgimages. We don't have a use case for that right now, but I don't see a good reason for the restriction.

@aviatesk
Copy link
Member

The problem with using an array in the compiler is that you can't use it in pkgimages.

Yeah, I agree.

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

Seems like a clearer way to handle it

@Keno Keno merged commit 5d80593 into master Jul 30, 2024
7 checks passed
@Keno Keno deleted the kf/typenameconstpropheuristic branch July 30, 2024 09:34
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024
…JuliaLang#55288)

As mentioned in JuliaLang#55271, I'd like to get rid of `istopfunction` for
various reasons. Rather than hardcoding constprop overrides based on
binding name, this annotates the relevant TypeNames (like the existing
`max_methods` override), removing the binding lookups from inference.
Keno added a commit that referenced this pull request Oct 23, 2024
This commit teaches to compiler to update its world bounds whenever
it looks at a binding partition, making the compiler sound in the
presence of a partitioned binding. The key adjustment is that the
compiler is no longer allowed to directly query the binding table
without recording the world bounds, so all the various abstract
evaluations that look at bindings need to be adjusted and are no
longer pure tfuncs. We used to look at bindings a lot more, but
thanks to earlier prep work to remove unnecessary binding-dependent
code (#55288, #55289 and #55271), these changes become relatively
straightforward.

Note that as before, we do not create any binding partitions by
default, so this commit is mostly preperatory.
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.

3 participants