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

LGTM

Ah, sorry, uploaded wrong file.

Addressed review comments

LGTM

Ping

Looks good at first sight. Do you have linker patch for me to try this out?

Could you possibly try check-all or check-llvm? This problem was occuring in the LLVM unit tests.

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).

• Added test case for 'readonly' constants (import-ro-constant.ll)
• Added support for 'Constant' attribute to dot dumper
• Addressed other comments

Out of curiosity, what problem do you run into when you unify ReadOnly with Constant?

I reimplemented the patch, now using extra 'Constant' attribute in GlobalVarSummary.

Patch was tested on Firefox and LLVM test suites. Both pass flawlessly

Found a bug, will resubmit after fix and more thorough tests.

LGTM

LGTM. BTW, do you have any data on performance improvement?

This also makes me think it might be a good idea to consider changing the ExportLists to hold the ValueInfos rather than GUIDs

But please send a follow on patch soon to do the assertion checking I suggested in my earlier comment

Addressed

Addressed

I'm a little confused about what causes the original failure mode

Rebased

Addressed and rebased

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.

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.

LGTM

Ping

Addressed comments.

Well, I found the original discussion here: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20120625/145646.html
Added @baldrick and @nlopes

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 .

@Bigcheese @chandlerc Can you please elaborate? I've run llvm, clang and lld test suites with lsan and everything seems good. What's the problem exactly?

Thanks for looking at it, Teresa!

Addressed

@tejohnson Teresa, are you ok with it?

Addressed

Added test case