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

Use at-consistent_overlay for 1.11 compatibility. #2492

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Sep 16, 2024

Address another part of #2241

@aviatesk I just realized though that we violate the requirements of @consistent_overlay without @device_function macro, which is used to implement intrinsics that are not CPU compatible (they make LLVM abort) so we make the origin method error()while the overlay performs the expected operation:

macro device_override(ex)
ex = macroexpand(__module__, ex)
esc(quote
Base.Experimental.@overlay(CUDA.method_table, $ex)
end)
end
macro device_function(ex)
ex = macroexpand(__module__, ex)
def = splitdef(ex)
# generate a function that errors
def[:body] = quote
error("This function is not intended for use on the CPU")
end
esc(quote
$(combinedef(def))
@device_override $ex
end)
end

IIUC this is not valid with @consistent_overlay. Do you have another suggestion to implement this in a way that's compatible with the effects system?

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.26%. Comparing base (991e23a) to head (30fe600).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2492      +/-   ##
==========================================
+ Coverage   73.06%   73.26%   +0.20%     
==========================================
  Files         157      157              
  Lines       14930    14930              
==========================================
+ Hits        10908    10938      +30     
+ Misses       4022     3992      -30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maleadt maleadt linked an issue Sep 16, 2024 that may be closed by this pull request
@aviatesk
Copy link

Exactly, I think using @consistent_overlay for @device_function would very likely lead to mis-compilation. Would it be possible to change @device_override to a 2-arg macro and simply have @device_function use @consistent instead of @consistent_overlay?

@maleadt
Copy link
Member Author

maleadt commented Sep 16, 2024

Yeah. Hopefully that doesn't lead to inference issues a la JuliaLang/julia#52938.

@maleadt maleadt marked this pull request as ready for review September 16, 2024 16:29
@maleadt maleadt force-pushed the tb/consistent_overlay branch from 1144588 to 30fe600 Compare September 16, 2024 16:29
@maleadt maleadt merged commit 1e1f4ef into master Sep 16, 2024
1 check was pending
@maleadt maleadt deleted the tb/consistent_overlay branch September 16, 2024 18:32
amontoison pushed a commit to amontoison/CUDA.jl that referenced this pull request Sep 18, 2024
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.

Support for Julia 1.11
2 participants