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

improve life with large tuples a little #16460

Merged
merged 4 commits into from
May 21, 2016
Merged

improve life with large tuples a little #16460

merged 4 commits into from
May 21, 2016

Conversation

carnaval
Copy link
Contributor

I put the cutoff mentioned in #15702 in there.
tuple _apply inlining was unlimited.
Otherwise it's mostly trying to avoid llvm scalarizing load/store of large aggregates. Instead it uses the memcpy intrinsics so it is lowered to a function call when the size exceeds some thresold.
It's not very systematic, I'm just going through examples I see with catastrophic IR explosion while throwing random large tuples around.
Without this patch, just passing a large tuple through to the constructor of a type like

immutable C
x :: NTuple{N,Int}
end

generates an amount IR linear wrt the size of the tuple.

I'm hoping this won't prevent optimizations on small sizes (we may want to add our own thresold for that if it happens). Anyway, I trust llvm more to break up a small memcpy than merge a lot of loads.

@carnaval
Copy link
Contributor Author

just for fun, before :

$ time (./julia -e 'code_native(i->i, (NTuple{2000,Int},))' | wc -l)
4010

real    0m25.366s
user    0m25.289s
sys 0m0.285s

after

$ time (./julia -e 'code_native(i->i, (NTuple{2000,Int},))' | wc -l)
20

real    0m0.497s
user    0m0.497s
sys 0m0.210s

{
assert(!v.isboxed);
if (v.ispointer())
return tbaa_decorate(v.tbaa, build_load(builder.CreatePointerCast(v.V, t->getPointerTo()), v.typ));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use data_pointer() instead of v.V (in case v is a constant)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't matter either then (since you already checked for isconstant)

@vtjnash vtjnash merged commit 5bef837 into master May 21, 2016
@vtjnash vtjnash deleted the ob/cgtup branch May 21, 2016 20:28
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.

3 participants