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

Move most Interval method bodies to the .cpp file #4830

Merged
merged 3 commits into from
Apr 5, 2020
Merged

Conversation

steven-johnson
Copy link
Contributor

Saves ~12k from libHalide.so on x86-64-linux; timing on local_laplacian.generator doesn't show any performance impact.

@abadams
Copy link
Member

abadams commented Apr 4, 2020

I'm surprised by this one. Most of these methods are incredibly cheap, e.g. two pointer comparisons. I had assumed they were being inlined. Can we hold off on merging this one until I can investigate?

@abadams
Copy link
Member

abadams commented Apr 4, 2020

Slows down allocation bounds inference on local laplacian by 5% or so. To claw it back I just had to inline both constructors. That in fact gave a minor speed boost (1%) over master for that stage.

Turns out operator== exists because the built-in autoscheduler wants to test to map<string, Interval> for equality in order to cache computations. I'll add a note.

@steven-johnson
Copy link
Contributor Author

No rush.

@steven-johnson
Copy link
Contributor Author

Huh, clearly I didn't time carefully enough, my local-laplacian run seemed to be within typical noise variation.

@abadams
Copy link
Member

abadams commented Apr 4, 2020

I just put tic/toc around allocation bounds inference, which is the stage that uses intervals the most. The 5%/1% refers to that stage alone. It is indeed in the noise for overall compilation.

@abadams abadams merged commit 60dac03 into master Apr 5, 2020
@abadams abadams deleted the srj-interval branch April 10, 2020 18:18
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