-
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
Add exceptions to check_ir_values #630
Conversation
Are there any other uses for double that are legal? If not, I'd rather we somehow whitelist the |
Use of |
Yeah, if it survives optimization. Or, alternatively, a function attribute ( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #630 +/- ##
==========================================
+ Coverage 71.34% 71.67% +0.33%
==========================================
Files 24 24
Lines 3186 3202 +16
==========================================
+ Hits 2273 2295 +22
+ Misses 913 907 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
2d5853a
to
4d99de5
Compare
4d99de5
to
809940d
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.
After some testing, I don't think metadata is going to work. In any case, it's the wrong tool for the job. We could devise more elaborate IR patterns that do survive, but I don't see the value in that.
Let's just not use check_ir_values
, which is a simple enough function, and instead roll our own version in metal.jl
which ignores the fpext
/store
. Or, if you feel fancy, maybe a allow_cb
callback argument to check_ir_values
.
Maybe add a simple test with pre-defined IR modules to check this behavior ( |
Allow
double
for C's default argument promotions used in variable-length argument lists by introducingir_check_ignore
metadata and a allow argument forcheck_ir_values
that is used to allowfpext
in Metal.needed for JuliaGPU/Metal.jl#418