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

RFC: lower T[a b; c d] to typed_hvcat(T, Val((2, 2)), a, b, c, d) #36719

Closed
wants to merge 1 commit into from

Conversation

simeonschaub
Copy link
Member

Please tell me if this is a dumb idea, but I'm doing some shenanigans with Base.typed_hvcat in https://github.com/simeonschaub/CoolTensors.jl, where this significantly helps constant propagation and therefore type inference. This should be entirely non-breaking, because I added a fallback definition that falls back to calling the old methods. Is there reason to be concerned about latencies and invalidations here? From my qualitative testing, I didn't notice any difference and am inclined to say that this shouldn't make much of a difference for array constructors anyways, but perhaps it might be a good idea to add @nospecialize to the fallback methods?

simeonschaub added a commit to simeonschaub/StaticArrays.jl that referenced this pull request Jul 18, 2020
@simeonschaub
Copy link
Member Author

It turns out, this actually benefits the SA constructor of StaticArrays as well: JuliaArrays/StaticArrays.jl#811

@MasonProtter
Copy link
Contributor

How would this work if something in the hvcat expression isn’t scalar? I.e. concatenating subarrays?

@simeonschaub
Copy link
Member Author

It still works:

julia> Int[1:3 [3, 6, 4]; 7 [9]]
4×2 Matrix{Int64}:
 1  3
 2  6
 3  4
 7  9

It's always this function that gets called after lowering and the tuple dims doesn't contain any information about whether the entry is an array or not, it's just how many objects are in each row.

@c42f
Copy link
Member

c42f commented Jul 20, 2020

This seems interesting, and I think it could make sense because it exposes the syntax explicitly as a type which libraries can rely on rather than forcing them to rely on constant propagation of rows if they want to construct a type which depends on rows.

However I'm also not entirely sure this is necessary; as noted in JuliaArrays/StaticArrays.jl#811 (comment), SA already seems to work as expected without any slowdown by just relying on constant propagation.

@JeffBezanson
Copy link
Member

I hope this can be avoided, since it will be harder on the compiler.

simeonschaub added a commit to simeonschaub/StaticArrays.jl that referenced this pull request Oct 27, 2020
@simeonschaub simeonschaub deleted the hvcat_val branch January 15, 2022 10:00
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.

4 participants