- User Since
- Sep 1 2015, 3:36 AM (228 w, 4 d)
Thu, Jan 16
I think that adding extra flag to GlobPattern::create was better idea. Just set it to false by default, so that GlobPattern will still act as glob matcher for all other users:
static static Expected<GlobPattern> create(StringRef Pat, bool IgnoreMetaChars = false);
Than add extra constructor to StringMatcher to which you'll pass GlobPattern and use it from InputSectionDescription constructor.
Thanks, I'm still in process of testing (now fixing issue which however is most likely related to devirtualization itself, not to this patch). Meanwhile some of my comments below.
I think this has to be rebased - I see multiple failures when trying to apply
Wed, Jan 15
Tue, Jan 14
Ah, sorry, uploaded wrong file.
Addressed review comments
Mon, Jan 13
Fri, Dec 27
Looks good at first sight. Do you have linker patch for me to try this out?
Dec 18 2019
Dec 17 2019
Could you possibly try check-all or check-llvm? This problem was occuring in the LLVM unit tests.
Dec 13 2019
Strange this assertion probably means, that there are two summaries with external linkage in VTableVI. I tried reproducer out of curiosity, but it worked fine for me (no assertion on stage2 with ninja clang).
Dec 12 2019
- Added test case for 'readonly' constants (import-ro-constant.ll)
- Added support for 'Constant' attribute to dot dumper
- Addressed other comments
Nov 22 2019
Out of curiosity, what problem do you run into when you unify ReadOnly with Constant?
Nov 21 2019
I reimplemented the patch, now using extra 'Constant' attribute in GlobalVarSummary.
Patch was tested on Firefox and LLVM test suites. Both pass flawlessly
Nov 19 2019
Found a bug, will resubmit after fix and more thorough tests.
Nov 18 2019
LGTM. BTW, do you have any data on performance improvement?
Nov 15 2019
Nov 14 2019
Nov 13 2019
This also makes me think it might be a good idea to consider changing the ExportLists to hold the ValueInfos rather than GUIDs
Nov 12 2019
Nov 8 2019
But please send a follow on patch soon to do the assertion checking I suggested in my earlier comment
I'm a little confused about what causes the original failure mode
Nov 7 2019
Nov 5 2019
Addressed and rebased
Nov 1 2019
Oct 31 2019
Adding new flag to summary index allowed better handling cases when actual dead symbol computation was skipped for some reason. See modifications in processGlobalForThinLTO and updated test cases.
Oct 30 2019
Addressed. See comment above for performance insights
I can see why this is a good idea, but do you have numbers for this optimization? How much code can be inlined?
This patch doesn't directly affect inlining, though it can reduce number of IR instructions and make inline cost of a function lower.
LLVM test suite shows noticeable code size reduction of Bullet test case (~1%), because deallocation function pointer sFreeFunc is imported with initializer
and load instruction gets eliminated in few destructors (indirect call is converted to direct call). On the whole the change introduced by this patch is quite small
and I wouldn't expect much from it. It would probably work better for C programs, where function pointers in structures is common OO pattern.
Oct 29 2019
Oct 28 2019
Oct 26 2019
Oct 25 2019
Well, I found the original discussion here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20120625/145646.html
Added @baldrick and @nlopes
Oct 24 2019
This was originally not added to the legacy pass manager because it leaks memory under it due to fundamental issues with the old pass manager. See https://reviews.llvm.org/D48105#1140299 .
Oct 23 2019
Thanks for looking at it, Teresa!
Oct 22 2019
Oct 21 2019
@tejohnson Teresa, are you ok with it?
Added test case