inference: followups for #51754 #52241
Conversation
aviatesk
added
compiler:inference
base/array.jl
Outdated
| @@ -130,9 +130,9 @@ to tell the compiler that indexing operations within the applied expression are | |||
| inbounds and do not need to taint `:consistent` and `:nothrow`. | |||
| """ | |||
| macro _safeindex(ex) | |||
There was a problem hiding this comment.
I don't really know where this macro came from, but the __safe_getindex function is very dangerous. Effect annotations need to be valid for all possible argument values. I'd like to limits its further propagation if possible. If we really need this, we could potentially think about call-site @assume_effects, but ugh.
There was a problem hiding this comment.
I think it comes from a desire to let f(a, i) = (@boundscheck boundscheck(a, i); @inbounds a[i]) have the same IPO effects as the combined operation f(a, i) = a[i], while still poisoning the independent expressions themselves (not consistent and ub, respectively) after inlining
There was a problem hiding this comment.
The macro @_safeindex was created to mimic the effect of @inbounds [getindex|setindex!] without affecting :noub. But, echoing Keno's point, a more effective approach might be to isolate the specific indexing code into its own function, and add effects annotations to that function:
I'd rather pull out this table lookup into a separate function along with the
jpcomputation and a long comment about why it's inbounds.
|
I'm not too happy with the last commit, but everything else if fine, so if you want to split the PR, those can go ahead and we can keep talking about the last one. |
aviatesk
force-pushed
the
avi/exception_types
branch
3 times, most recently
from
9367cc2
to
6723c80
Compare
This commit defines functions that mirror our tools for analyzing return types and computational effects. The key point to discuss is that this commit introduces two functions: `Base.exception_types` and `Base.exception_type`. `Base.exception_types` acts like `Base.return_types`, giving a list of exception types for each method that matches with the given call signature. On the other hand, `Base.exception_type` is akin to `Base.infer_effects`, returning a single exception type that covers all potential outcomes entailed by the given call signature. I personally lean towards the latter for its utility, particularly in testing scenarios, but I included `exception_types` too for consistency with `return_types`. I'd welcome any feedback on this approach.
aviatesk
force-pushed
the
avi/exception_types
branch
from
6723c80
to
15ab026
Compare
aviatesk
force-pushed
the
avi/follow-51754
branch
from
525bd6c
to
8dd0cf5
Compare
|
@nanosoldier |
|
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. |
Continued from #52241. This PR focuses on improving exception type inference for core math functions by using new callsite `Base.@assume_effects` annnotation.
Composed of:
update_cycle_worklists!utility (78f7b4e)typenamecall (fac36d8):nothrowis proven (76143d3)improve exception type inference for core math functions (525bd6c)Separated into TODO