-
-
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
effects: taint overlay-ed method's :nonoverlayed
effect bit
#51078
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aviatesk
added a commit
that referenced
this pull request
Aug 28, 2023
Certain external `AbstractInterpreter`s like GPUCompiler.jl have long been wanting to allow concrete eval for certain overlay-ed methods to get a best inference accuracy, although it is currently prohibited. It should be safe when an overlay-ed method has the same semantics as the original method and its result can be safely replaced by the result of the original method. See JuliaGPU/GPUCompiler.jl#384 for examples. To address this issue, this commit allows `@assume_effects`-override of `:nonoverlayed` effect bit. On top of #51078, the override of `:nonoverlayed` works as like the other effect bits, and so external `AbstractInterpreter` can use it to allow concrete eval of annotated overlay-ed methods: ```julia @overlay OVERLAY_MT Base.@assume_effects :nonoverlayed f(x) = [...] ``` Now it sounds awkward to annotate `Base.@assume_effects :nonoverlayed` on `@overlay`-ed method. We likely want to rename `:nonoverlayed` to `native_executable` or something more reasonable. Certain external `AbstractInterpreters`, such as GPUCompiler.jl, have long sought the ability to allow concrete evaluation for specific overlay-ed methods to achieve optimal inference accuracy. This is currently not permitted, although it should be safe when an overlay-ed method has the same semantics as the original method, and its result can be safely replaced with the result of the original method. Refer to JuliaGPU/GPUCompiler.jl#384 for more examples. To address this issue, this commit introduces the capability to override the `:nonoverlayed` effect bit using `@assume_effects`. With the enhancements in PR #51078, this override behaves similarly to other effect bits. Consequently, external `AbstractInterpreters` can utilize this feature to permit concrete evaluation for annotated overlay-ed methods, e.g. `@overlay OVERLAY_MT Base.@assume_effects :nonoverlayed f(x) = [...]`. However, it now seems awkward to annotate a method with `Base.@assume_effects :nonoverlayed` when it is actually marked with `@overlay`. A more intuitive terminology, like `native_executable`, might be more appropriate for renaming the `:nonoverlayed` effect bit.
aviatesk
added a commit
that referenced
this pull request
Aug 28, 2023
Certain external `AbstractInterpreters`, such as GPUCompiler.jl, have long sought the ability to allow concrete evaluation for specific overlay-ed methods to achieve optimal inference accuracy. This is currently not permitted, although it should be safe when an overlay-ed method has the same semantics as the original method, and its result can be safely replaced with the result of the original method. Refer to JuliaGPU/GPUCompiler.jl#384 for more examples. To address this issue, this commit introduces the capability to override the `:nonoverlayed` effect bit using `@assume_effects`. With the enhancements in PR #51078, this override behaves similarly to other effect bits. Consequently, external `AbstractInterpreters` can utilize this feature to permit concrete evaluation for annotated overlay-ed methods, e.g. ```julia @overlay OVERLAY_MT Base.@assume_effects :nonoverlayed f(x) = [...] ``` However, it now seems awkward to annotate a method with `Base.@assume_effects :nonoverlayed` when it is actually marked with `@overlay`. A more intuitive terminology, like `native_executable`, might be more appropriate for renaming the `:nonoverlayed` effect bit.
Keno
approved these changes
Aug 28, 2023
Previously we tainted `:nonoverlayed` bit of the callers of overlay-ed methods by looking at the method match results, rather than tainting the overlay-ed methods' effects themselves. This is a bit confusing since it is not aligned with how the other effect bits are tainted. Moreover, I am planning to allow `Base.@assume_effects`-override for `:nonoverlayed` effect bit in the future to solve issues like JuliaGPU/GPUCompiler.jl#384, and it would be necessary for the solution to be functional that `:nonoverlayed` effect bit is tainted on the callee-side as the other effect bits are. This commit refactors the compiler internal so that we taint `:nonoverlayed` bit of overlay-ed methods and propagate it to callers. It turns out that this refactor simplifies the internal implementations a lot.
aviatesk
force-pushed
the
avi/nonoverlayed-effects
branch
from
August 29, 2023 03:53
ad24c70
to
00b5060
Compare
aviatesk
added a commit
that referenced
this pull request
Aug 29, 2023
Certain external `AbstractInterpreters`, such as GPUCompiler.jl, have long sought the ability to allow concrete evaluation for specific overlay-ed methods to achieve optimal inference accuracy. This is currently not permitted, although it should be safe when an overlay-ed method has the same semantics as the original method, and its result can be safely replaced with the result of the original method. Refer to JuliaGPU/GPUCompiler.jl#384 for more examples. To address this issue, this commit introduces the capability to override the `:nonoverlayed` effect bit using `@assume_effects`. With the enhancements in PR #51078, this override behaves similarly to other effect bits. Consequently, external `AbstractInterpreters` can utilize this feature to permit concrete evaluation for annotated overlay-ed methods, e.g. ```julia @overlay OVERLAY_MT Base.@assume_effects :nonoverlayed f(x) = [...] ``` However, it now seems awkward to annotate a method with `Base.@assume_effects :nonoverlayed` when it is actually marked with `@overlay`. A more intuitive terminology, like `native_executable`, might be more appropriate for renaming the `:nonoverlayed` effect bit.
aviatesk
added a commit
that referenced
this pull request
Aug 30, 2023
Certain external `AbstractInterpreters`, such as GPUCompiler.jl, have long sought the ability to allow concrete evaluation for specific overlay-ed methods to achieve optimal inference accuracy. This is currently not permitted, although it should be safe when an overlay-ed method has the same semantics as the original method, and its result can be safely replaced with the result of the original method. Refer to JuliaGPU/GPUCompiler.jl#384 for more examples. To address this issue, this commit introduces the capability to override the `:nonoverlayed` effect bit using `@assume_effects`. With the enhancements in PR #51078, this override behaves similarly to other effect bits. Consequently, external `AbstractInterpreters` can utilize this feature to permit concrete evaluation for annotated overlay-ed methods, e.g. ```julia @overlay OVERLAY_MT Base.@assume_effects :nonoverlayed f(x) = [...] ``` However, it now seems awkward to annotate a method with `Base.@assume_effects :nonoverlayed` when it is actually marked with `@overlay`. A more intuitive terminology, like `native_executable`, might be more appropriate for renaming the `:nonoverlayed` effect bit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we tainted
:nonoverlayed
bit of the callers of overlay-ed methods by looking at the method match results, rather than tainting the overlay-ed methods' effects themselves. This is a bit confusing since it is not aligned with how the other effect bits are tainted.Moreover, I am planning to allow
Base.@assume_effects
-override for:nonoverlayed
effect bit in the future to solve issues like JuliaGPU/GPUCompiler.jl#384, and it would be necessary for the solution to be functional that:nonoverlayed
effect bit is tainted on the callee-side as the other effect bits are.This commit refactors the compiler internal so that we taint
:nonoverlayed
bit of overlay-ed methods and propagate it to callers. It turns out that this refactor simplifies the internal implementations a lot.