[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
AaronBallman wrote: The precommit CI failures are real; I'm still investigating what's going sideways. https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
@@ -1163,7 +1137,7 @@ class Sema; ConversionSequenceList allocateConversionSequences(unsigned NumConversions) { ImplicitConversionSequence *Conversions = - slabAllocate(NumConversions); + Ctx.Allocate(NumConversions); // Construct the new objects. for (unsigned I = 0; I != NumConversions; ++I) cor3ntin wrote: Maybe something like that would be cleaner ```cpp ConversionSequenceList allocateConversionSequences(unsigned NumConversions) { ImplicitConversionSequence *Conversions = Ctx.Allocate(NumConversions); std::uninitialized_default_construct(Conversions, Conversions + NumConversions); return ConversionSequenceList(Conversions, NumConversions); } ``` https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
@@ -1070,57 +1077,24 @@ class Sema; }; private: -SmallVector Candidates; -llvm::SmallPtrSet Functions; - -// Allocator for ConversionSequenceLists. We store the first few of these -// inline to avoid allocation for small sets. -llvm::BumpPtrAllocator SlabAllocator; +ASTContext &Ctx; +SmallVector Candidates; +llvm::SmallPtrSet Functions; mizvekov wrote: pre-existing small nit, for consistency: ```suggestion SmallVector Candidates; SmallPtrSet Functions; ``` https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
@@ -1070,57 +1077,24 @@ class Sema; }; private: -SmallVector Candidates; -llvm::SmallPtrSet Functions; - -// Allocator for ConversionSequenceLists. We store the first few of these -// inline to avoid allocation for small sets. -llvm::BumpPtrAllocator SlabAllocator; +ASTContext &Ctx; +SmallVector Candidates; +llvm::SmallPtrSet Functions; AaronBallman wrote: The reason it works to drop the `llvm::` prefix for `SmallVector` is because we do this: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LLVM.h `SmallPtrSet` is not included in there which is why the prefix is necessary, and I'm not certain it's worth including it or not. Thoughts? https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
https://github.com/cor3ntin approved this pull request. This was a roller coaster! LGTM https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
@@ -1070,57 +1077,24 @@ class Sema; }; private: -SmallVector Candidates; -llvm::SmallPtrSet Functions; - -// Allocator for ConversionSequenceLists. We store the first few of these -// inline to avoid allocation for small sets. -llvm::BumpPtrAllocator SlabAllocator; +ASTContext &Ctx; +SmallVector Candidates; +llvm::SmallPtrSet Functions; mizvekov wrote: I see, it's fine either way 😃 https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
vitalybuka wrote: A new memory leak https://lab.llvm.org/buildbot/#/builders/5/builds/42694 https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
nikic wrote: This change seems to cause significant compile-time regressions for C++ code (https://llvm-compile-time-tracker.com/compare.php?from=184ba038ac1d444980b3e554b0057f3f30c516ab&to=4082a7554521572a65a5a0008c4661a534df659d&stat=instructions%3Au). Probably most damning are the times for the clang build itself (https://llvm-compile-time-tracker.com/compare_clang.php?from=184ba038ac1d444980b3e554b0057f3f30c516ab&to=4082a7554521572a65a5a0008c4661a534df659d&stat=instructions%3Au&sortBy=interestingness) where files like X86ISelDAGToDAG.cpp regress by >3% even in an optimized build. https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
vitalybuka wrote: have ideas about leak, but not sure what to do with perf regression and #88330 Proposing revert https://github.com/llvm/llvm-project/pull/89006 https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
@@ -1163,12 +1136,7 @@ class Sema; ConversionSequenceList allocateConversionSequences(unsigned NumConversions) { ImplicitConversionSequence *Conversions = - slabAllocate(NumConversions); - - // Construct the new objects. - for (unsigned I = 0; I != NumConversions; ++I) -new (&Conversions[I]) ImplicitConversionSequence(); - + new ImplicitConversionSequence[NumConversions]; vitalybuka wrote: Seems unusual to keep ownership in ArrayRef. Could we make ConversionSequenceList just a std::vector if it owns the memory? https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
cor3ntin wrote: @AaronBallman ping https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
AaronBallman wrote: > @AaronBallman ping I've not been able to get back on to this, unfortunately. I'm not certain when I'll have time, either. https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Improve stack usage to increase recursive initialization depth (PR #88546)
cor3ntin wrote: It might be worth reopening the PR and asking for help? otherwise we'll forget *And some things that should not have been forgotten were lost.* https://github.com/llvm/llvm-project/pull/88546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits