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.

Sign up for GitHub

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

Jump to bottom

Carry patch to avoid excessive LLVM time #48681

Closed
Keno opened this issue Feb 15, 2023 · 6 comments
Closed

Carry patch to avoid excessive LLVM time #48681

Keno opened this issue Feb 15, 2023 · 6 comments
Assignees
@staticfloat @vchuravy
Labels
compiler:latency Compiler latency compiler:llvm For issues that relate to LLVM domain:building Build system, or building Julia or its dependencies external dependencies Involves LLVM, OpenBLAS, or other linked libraries

Comments

@Keno
Copy link
Member

Keno commented Feb 15, 2023

We should carry the patch in llvm/llvm-project#60132. It doesn't look like there's a good patch forthcoming any time soon and the optimization that gets disabled there is minor. @vchuravy can you do this with the next LLVM bump?
@giordano giordano added domain:building Build system, or building Julia or its dependencies external dependencies Involves LLVM, OpenBLAS, or other linked libraries compiler:llvm For issues that relate to LLVM labels Feb 15, 2023
@gbaraldi
Copy link
Member

gbaraldi commented Feb 15, 2023 •
edited
Loading

There's a comment here

hasPredecessorHelper already takes an optional MaxSteps limit that we could make use of

Could we keep the optimization put not be pathological about it? Or is the case that even with limiting that we still get bad performance?

@vchuravy
Copy link
Sponsor Member

Do you need this backported to 1.9?

@vchuravy vchuravy self-assigned this Feb 15, 2023
@oscardssmith
Copy link
Member

no.

@Keno
Copy link
Member Author

Keno commented Feb 15, 2023

Could we keep the optimization put not be pathological about it? Or is the case that even with limiting that we still get bad performance?

Maybe, but I would prefer just a clean disable. The optimization is minimal and the cost is extreme.

@JeffBezanson JeffBezanson added the compiler:latency Compiler latency label Feb 16, 2023
@pepijndevos
Copy link
Contributor

Is this just a matter of dunking the patch in the right folder or more involved? We can't really run some tests in CI until this gets done.

@staticfloat staticfloat self-assigned this Mar 23, 2023
@staticfloat
Copy link
Sponsor Member

X-ref: JuliaLang/llvm-project#14

@vchuravy vchuravy mentioned this issue Apr 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@staticfloat staticfloat

@vchuravy vchuravy

Labels
compiler:latency Compiler latency compiler:llvm For issues that relate to LLVM domain:building Build system, or building Julia or its dependencies external dependencies Involves LLVM, OpenBLAS, or other linked libraries
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@staticfloat @vchuravy @pepijndevos @JeffBezanson @giordano @Keno @oscardssmith @gbaraldi