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

Ensure that ArrayLS and ArrayLSLS really are LinearSlow #16

Merged
merged 1 commit into from
Jul 12, 2016
Merged

Conversation

timholy
Copy link
Member

@timholy timholy commented Jul 11, 2016

Our inlining and codegen are getting so dang awesome on julia 0.5 that recently LLVM has begun to notice that it can skip the div when performing linear indexing ArrayLS and ArrayLSLS. Since testing that is not really the intention of these tests, I modified the tests to defeat it. Ref JuliaLang/julia#17355 (comment).

B = similar(A, r+1, c+1)
B[1:r, 1:c] = A
AS = ArrayLS(B)
ASS = ArrayLSLS(B)
Copy link
Member

@jrevels jrevels Jul 11, 2016

Choose a reason for hiding this comment

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

ASS = ArrayLSLS(B)

😯 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe we should change those names 😄

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I didn't notice it before...maybe it's now an immortal piece of Julia benchmarking history? I'll leave it up to you to decide. 😁

@timholy
Copy link
Member Author

timholy commented Jul 11, 2016

The removal of UTF16String from base seems to have broken JLD (I see what you mean...), so this won't pass on 0.5.

@timholy
Copy link
Member Author

timholy commented Jul 12, 2016

Since JLD was tagged and all of this passes, I'm going to merge (I want to test a PR 😉).

@timholy timholy merged commit 492def3 into master Jul 12, 2016
@timholy timholy deleted the teh/LS branch July 12, 2016 13:15
@timholy
Copy link
Member Author

timholy commented Jul 12, 2016

Out of curiosity, did https://github.com/JuliaCI/BaseBenchmarkReports/blob/9dcaa7b11fc1ab036c06ef34d488d55838e31d40/019ba50_vs_f881efb/report.md run with this, or without it? It's exactly the pattern I would have predicted if this was not included.

@jrevels
Copy link
Member

jrevels commented Jul 12, 2016

Ah, sorry. Execution parameters have to be retuned before new benchmarks (or changes) land in the nanosoldier branch. I was going to run it tomorrow, but I can start the retuning process now instead. It usually takes a couple of hours.

@timholy
Copy link
Member Author

timholy commented Jul 12, 2016

No hurry, I've checked locally and there doesn't seem to be a problem with that PR.

Thanks for explaining, it's nice to know the inner mysteries of the universe 🎱

jrevels referenced this pull request in JuliaLang/julia Jul 14, 2016
RFC: Remove command-line Git from Mac and Windows binaries
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.

2 participants