-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Make `cat` inferrable even if its arguments are not fully constant: ```julia julia> r = rand(Float32, 56, 56, 64, 1); julia> f(r) = cat(r, r, dims=(3,)) f (generic function with 1 method) julia> @inferred f(r); julia> last(@code_typed f(r)) Array{Float32, 4} ``` After descending into its call graph, I found that constant propagation is prohibited at `cat_t(::Type{T}, X...; dims)` due to the method instance heuristic, i.e. its body is considered to be too complex for successful inlining although it's explicitly annotated as `@inline`. But for this case, the constant propagation is greatly helpful both for abstract interpretation and optimization since it can improve the return type inference. Since it is not an easy task to improve the method instance heuristic, which is our primary logic for constant propagation, this commit does a quick fix by helping inference with the `@constprop` annotation. There is another issue that currently there is no good way to properly apply `@constprop`/`@inline` effects to a keyword function (as a note, this is a general issue of macro annotations on a method definition). So this commit also changes some internal helper functions of `cat` so that now they are not keyword ones: the changes are also necessary for the `@inline` annotation on `cat_t` to be effective to trick the method instance heuristic.
- Loading branch information
Showing
4 changed files
with
24 additions
and
32 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65b9be4
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.
Executing the daily package evaluation, I will reply here when finished:
@nanosoldier
runtests(ALL, isdaily = true)
65b9be4
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.
Your package evaluation job has completed - possible new issues were detected. A full report can be found here.