nikic reopened this revision. nikic added a comment. This revision is now accepted and ready to land.
I have reverted this change, because it causes a 1% compile-time <http://llvm-compile-time-tracker.com/compare.php?from=73b7dd1fb3c17a4ac4b1f1e603f26fa708009649&to=b8d08e961df1d229872c785ebdbc8367432e9752&stat=instructions> and memory usage <http://llvm-compile-time-tracker.com/compare.php?from=73b7dd1fb3c17a4ac4b1f1e603f26fa708009649&to=b8d08e961df1d229872c785ebdbc8367432e9752&stat=max-rss> regression. The memory usage regression is probably fine given what this change does, but the compile-time regression is not. (For context, this pretty much undoes the wins that the recent removal of waymarking gave us.) Some notes: - Can you please split out the fix to grow() into a separate revision? It does not seem related to the main change, and reduces surface area. - I don't think the automatic switch of the size/capacity field has been justified well. We have plenty of SmallVectors in LLVM that are, indeed, small. There is no way an MCRelaxableFragment will ever end up storing a single instruction that is 4G large. - Similarly, I'm not really convinced about handling this in SmallVector at all. The original change here just used an `std::vector` in the one place where this has become an issue. That seems like a pretty good solution until there is evidence that this is really a more widespread problem. But in any case, my primary concern here is the compile-time regression, and it's not immediately clear which part of the change it comes from. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77621/new/ https://reviews.llvm.org/D77621 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
