-
Notifications
You must be signed in to change notification settings - Fork 53
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
Overlay methods disabling IR interpreter breaks const-prop of GPU-incompatible code #384
Comments
The conversion of Irrational to Float32 didn't meaningfully change, so I assume this is related to changing MWE: julia> f(x) = Float32(x,RoundDown)
julia> code_llvm(f, Tuple{Irrational{:π}})
; @ REPL[6]:1 within `f`
define float @julia_f_175() #0 {
top:
ret float 0x400921FB40000000
}
julia> CUDA.code_llvm(f, Tuple{Irrational{:π}})
; @ REPL[6]:1 within `f`
define float @julia_f_496() local_unnamed_addr #0 {
top:
; ┌ @ irrationals.jl:69 within `Type`
; │┌ @ mpfr.jl:960 within `setprecision`
%0 = call fastcc float @julia__setprecision_25_499() #0
; └└
ret float %0
} Fixing this isn't trivial. We probably need some more subtle notion of method overlays, where we vouch that the replacement method has the same behavior and thus the Julia compiler is allowed to interpret this IR using the original methods. @aviatesk Is my understanding correct here? |
Another example is JuliaLang/julia#48097, where a |
Yeah, I guess most overlays used by GPU stack is safe to be ignored during compilation assuming those overlays aren't supposed to change the semantics of the original methods. |
We can close this issue now? |
The underlying issue is still there, right? We just worked around |
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.
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.
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.
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.
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.
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.
Moving the discussion upstream: JuliaLang/julia#52940 |
Describe the bug
When comparing irrational numbers, the kernel compilation throws an InvalidIRError.
This does not happen in Julia 1.8, only in Julia 1.9.
To reproduce
The Minimal Working Example (MWE) for this bug:
However, explicit conversion works:
I am using a temp environment where only CUDA is installed:
Expected behavior
I get the expected result without errors in Julia 1.8:
Version info
Details on Julia:
Details on CUDA:
Additional context
Full Stacktrace:
The text was updated successfully, but these errors were encountered: