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

Add extra early memcpyopt pass #22049

Merged
merged 1 commit into from
May 25, 2017
Merged

Add extra early memcpyopt pass #22049

merged 1 commit into from
May 25, 2017

Conversation

Keno
Copy link
Member

@Keno Keno commented May 24, 2017

Under certain circumstances, we emit loads/stores of large LLVM
structs. A lot of these can be trivially folded to memcpy and memcpyopt
is capable of doing so, but we weren't running it until after SROA.
SROA unfortunately, likes to take these apart, causing exponential
compile-time blow up and reduced runtime performance. In one particular
case (2000 element tuple), this change results in a 100x improvement
in compile time.

Under certain circumstances, we emit loads/stores of large LLVM
structs. A lot of these can be trivially folded to memcpy and memcpyopt
is capable of doing so, but we weren't running it until after SROA.
SROA unfortunately, likes to take these apart, causing exponential
compile-time blow up and reduced runtime peroformance. In one particular
case (2000 element tuple), this change results in a 100x improvement
in compile time.
@Keno Keno merged commit 8b2cac4 into master May 25, 2017
@martinholters martinholters deleted the kf/memcpyopt branch May 25, 2017 19:40
@alyst
Copy link
Contributor

alyst commented May 26, 2017

Could this PR be the reason for Nullable regressions in the latest daily benchmark?

@KristofferC
Copy link
Member

Really should have run nanosoldier here when it is a PR that explicitly deals with performance.

@Keno
Copy link
Member Author

Keno commented May 30, 2017

Confirmed to have been this change. It's a bug in the loop vectorizer's cost model. However, the issue is resolved on LLVM master. Since we're not gonna release 0.7 before upgrading LLVM, I think we're fine.

@yuyichao
Copy link
Contributor

Any link to the bug fix on LLVM master?

@Keno
Copy link
Member Author

Keno commented May 30, 2017

Haven't tracked down the exact commit. If you really care I'll run a bisect.

@yuyichao
Copy link
Contributor

Just wondering if it's clean enough to be backported to 3.9/4.0. A bisect would be cool but no hurry.

@Keno
Copy link
Member Author

Keno commented May 30, 2017

Bisect running

@Keno
Copy link
Member Author

Keno commented May 30, 2017

Bisect claims llvm-mirror/llvm@371d289, but it found a different commit the first time I ran it, so I think whether this vectorizes or not has been going back and forth a bit.

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.

5 participants