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

Please cherry-pick several debug-info patches for llvm-14-rc1 #53548

Closed
jmorse opened this issue Feb 2, 2022 · 12 comments
Closed

Please cherry-pick several debug-info patches for llvm-14-rc1 #53548

jmorse opened this issue Feb 2, 2022 · 12 comments

Comments

@jmorse
Copy link
Member

jmorse commented Feb 2, 2022

Hi Tom / release-engineering team,

I've been working on a debug-info variable location feature which missed the llvm-14 branch by a few hours. It was enabled on main for x86_64 for 2 weeks without much trouble, but I turned it off after receiving a reproducer for excessive memory consumption, for which I now have patches. I've got several commits that would need to be cherry picked (below), would you be alright with this being
in llvm14-rc1?

A few faults reported from the initial landing are here, https://reviews.llvm.org/D116821, which I can summarise if needs be, but they're all now fixed. I ended up disabling the feature after some auto-generated code in https://reviews.llvm.org/D116821#3278089 led to a 5x increase in memory consumption, fixed by https://reviews.llvm.org/D118601 adding an upper bound on tracking too much debug-info.

The patches to cherry-pick in would be:

  • 14aaaa1 (Re-)Apply max an upper-bound on the amount of debug-info to track,

  • d556eb7 A performance patch (memoize some queries)

  • a80181a Early-free some data structures

  • 9fd9d56 Optimise an algorithm to reduce max-rss

  • 206cafb Follow up to the above for an accident with asan

  • 43de305 Fix for a crash introduced by an unrelated performance opt

  • 6e03a68 Re-enable this feature by default for x86_64

The last of these just landed a few minutes ago, so it's worth leaving things ~24h to settle on main. As mentioned, it was on for 2 weeks beforehand without any major incidents.

(If needs be I can cherry-pick -x these to the release branch myself, exactly what the protocol is before -rc1 is unclear to me).

@jmorse
Copy link
Member Author

jmorse commented Feb 3, 2022

NB: I've put all the cherry picks on this branch: https://github.com/jmorse/llvm-project/tree/release-14-instr-ref , for ease of access if these get merged.

Note that this also features 4654fa8, a temporary fix to keep asan happy while D118774 is finished. It's a very shallow object lifetime problem that I wasn't paying enough attention to, rather than a deep-seated issue.

@tstellar
Copy link
Collaborator

tstellar commented Feb 5, 2022

/cherry-pick 14aaaa1 d556eb7 a80181a 9fd9d56 206cafb 43de305 6e03a68

@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2022

/branch llvmbot/llvm-project/issue53548

@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2022

/pull-request llvmbot#8

@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2022

@llvm/issue-subscribers-debuginfo

@jmorse
Copy link
Member Author

jmorse commented Feb 7, 2022

For the avoidance of doubt: 4654fa8 will also be required to avoid a memory leak found by LeakSanitizer. As mentioned in my previous comment there's a generalised solution to this problem in D118774 being developed, but that's likely to only be ready for -rc2.

@tstellar
Copy link
Collaborator

tstellar commented Feb 7, 2022

For the avoidance of doubt: 4654fa8 will also be required to avoid a memory leak found by LeakSanitizer. As mentioned in my previous comment there's a generalised solution to this problem in D118774 being developed, but that's likely to only be ready for -rc2.

My mistake, I did not realize 4654fa8 was upstream. I will cherry-pick that one too.

@tstellar
Copy link
Collaborator

tstellar commented Feb 7, 2022

/cherry-pick 4654fa8

@tstellar
Copy link
Collaborator

tstellar commented Feb 7, 2022

@tstellar
Copy link
Collaborator

tstellar commented Feb 7, 2022

/branch llvmbot/llvm-project/issue53548

@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2022

/pull-request llvmbot#12

@tstellar
Copy link
Collaborator

tstellar commented Feb 8, 2022

Merged: 54a8365

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants