-
Notifications
You must be signed in to change notification settings - Fork 52
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
New deferred_codegen implementation #582
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #582 +/- ##
=======================================
Coverage 0.00% 0.00%
=======================================
Files 24 24
Lines 3064 3190 +126
=======================================
- Misses 3064 3190 +126 ☔ View full report in Codecov by Sentry. |
Okay this looks like the right direction Using:
This gets refined from
to
but codegen doesn't like what we are doing and generates a
|
Okay much nicer instead of refining to a Julia function we go straight to a llvmcall
@maleadt added benefit is that this should handle invalidations of |
b515f11
to
43adce8
Compare
@maleadt I left the old implementation alive since Enzyme is using it. We could add a token to declare who owns it... The addition of Potentially Enzyme ought to have a |
But this now sucessfully turns
Into
From:
|
Why does Enzyme always require horrible things... Can't this just be a breaking release? |
I wonder that myself... Yeah we can make this a breaking release, but I will need some more time to figure out how the Enzyme part will work. |
1b2439d
to
958bec7
Compare
@maleadt The Any ideas when that sanitation is happening? |
Ah it's probably Line 52 in 3c80a5d
|
d2d7a73
to
e532812
Compare
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.
LGTM! Rebased and cleaned-up a little.
How is this looking from the Enzyme.jl side, do we need to keep the old implementation around for now?
927fc30
to
b54b5e4
Compare
f54410e
to
4f14a5e
Compare
b37223e
to
5281e86
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
minfo = info.info | ||
results = minfo.results | ||
if length(results.matches) != 1 | ||
return nothing | ||
end | ||
match = only(results.matches) |
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.
Needs to be updated for 1.12
5281e86
to
ced39bb
Compare
Draft implementation #581
@aviatesk pointed me to:
For the refinement. So we will need a small pass that rewrites the call, post abstract interpretation.