[ThinLTO] Add noRecurse and noUnwind thinlink function attribute propagation

[modimo]

Summary

Thinlink provides an opportunity to propagate function attributes across modules, enabling additional propagation opportunities.

This change propagates (currently default off, turn on with disable-thinlto-funcattrs=1) noRecurse and noUnwind based off of function summaries of the prevailing functions in bottom-up call-graph order. Testing on clang self-build:

1. There's a 35-40% increase in noUnwind functions due to the additional propagation opportunities.
2. Throughput is measured at 10-15% increase in thinlink time which itself is 1.5% of E2E link time.

Implementation-wise this adds the following summary function attributes:

1. noUnwind: function is noUnwind
2. mayThrow: function contains a non-call instruction that Instruction::mayThrow returns true on (e.g. windows SEH instructions)
3. hasUnknownCall: function contains calls that don't make it into the summary call-graph thus should not be propagated from (e.g. indirect for now, could add no-opt functions as well)

Testing:
Clang self-build passes and 2nd stage build passes check-all
ninja check-all with newly added tests passing

[ncharlie]

lib/Transforms/IPO/FunctionImport.cpp

434 ↗

(On Diff #111769)

This is currently failing a test case that uses printf. Since printf doesn't have a summary associated with it (i.e. the SummaryList is empty), I can't determine if it recurses and have to fail out early.

Is there some spot I should add code to create a FunctionSummary for external functions?

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

434 ↗

(On Diff #111769)

How is printf handled in the full LTO case? Does it have attributes indicating that it is no recurse? Otherwise I think treating it conservatively is the only option.

[ncharlie]

lib/Transforms/IPO/FunctionImport.cpp

434 ↗

(On Diff #111769)

In FullLTO it's able to determine that printf doesn't recurse by analyzing the callsite (http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionAttrs.cpp#1068). Maybe I could modify ModuleSummaryAnalysis so it adds inserts the flags into the FunctionSummary while it still has access to the CallSite?

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

434 ↗

(On Diff #111769)

So then that means something is able to mark the external function call as NoRecurse. Maybe because it is a known libcall.

The question is what would you mark in the summary during ModuleSummaryAnalysis? Specifically, if the same function containing the call to a NoRecurse external function contained another call to a different function (external or not) that is not NoRecurse, the caller's function summary flags will conservatively not say NoRecurse.

So in that case it does seem useful to have function summaries for external functions that contain this info. However, that might have effects in other places. My suggestion is to be conservative now, and we can think about making that enhancement as a follow-on. (Unless you had something different in mind - let me know as I haven't thought through this too closely for awhile)

[ncharlie]

Skip functions that don't have a FunctionSummary. Return an empty root if a callgraph root can't be found.

[ncharlie]

lib/Transforms/IPO/FunctionImport.cpp

434 ↗

(On Diff #111769)

Ok, I've added logic to skip over functions that don't have a FunctionSummary associated.

[tejohnson]

Sorry again for the delay on the reviews, not sure how I missed that they were ready. Feel free to ping me much more frequently if I don't respond - weekly is good!

include/llvm/IR/ModuleSummaryIndex.h

662 ↗

(On Diff #128852)

Remove braces. Also, does this need to be part of D36311?

lib/LTO/LTO.cpp

1051 ↗

(On Diff #111577)

ThinLTOCodeGenerator is used by ld64 and other linkers that use the legacy LTO interface, and can be tested using llvm-lto. We should have these calls for both LTO APIs. I think earlier you had them only in ThinLTOCodeGenerator, and I asked to add them here, but they got removed from there in the process. If you can put it back then your test can add llvm-lto based testing lines as well. Unfortunately we still have these two LTO interfaces and need to keep them in sync.

lib/Transforms/IPO/FunctionImport.cpp

428 ↗

(On Diff #128852)

Add a TODO here to consider adding summaries for external nodes to hold function attribute information.

436 ↗

(On Diff #128852)

I assume we skip when NoRecurse is already true because it means it was already analyzed? If so, add comment to that effect.

442 ↗

(On Diff #128852)

Isn't a node with a GUID of 0 the dummy root node (as per comment above)? Would we ever have an edge to it?

453 ↗

(On Diff #128852)

Above we already returned if this flag was true, so I think this if check is unnecessary.

[modimo]

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

[tejohnson]

In D36850#2792474, @modimo wrote:

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

The patch has been dormant since @ncharlie hasn't been active in LLVM for awhile, and no one else has had a chance to pick it up. Would be great if you would like to take it over and push it forward.

[modimo]

In D36850#2795245, @tejohnson wrote:

In D36850#2792474, @modimo wrote:

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

The patch has been dormant since @ncharlie hasn't been active in LLVM for awhile, and no one else has had a chance to pick it up. Would be great if you would like to take it over and push it forward.

Can do, I'll send out an updated revision. Should I directly edit this diff or create a new one linked back?

[tejohnson]

In D36850#2795248, @modimo wrote:

In D36850#2795245, @tejohnson wrote:

In D36850#2792474, @modimo wrote:

@tejohnson Do you know more about the status of this patch/@ncharlie? I'd like to help push this forward and can take over if they're no longer available.

The patch has been dormant since @ncharlie hasn't been active in LLVM for awhile, and no one else has had a chance to pick it up. Would be great if you would like to take it over and push it forward.

Can do, I'll send out an updated revision. Should I directly edit this diff or create a new one linked back?

Hmm, maybe best if you do a Commandeer Revision and take this one over, that way we keep the comment history.

[modimo]

Rebased, cleaned up and updated implementation. Also fixed up all the tests to be correct. Note that currently lld/COFF tests fail from LLD_IN_TEST=2 which runs tests in memory twice. I checked that this also fails from doing SCC traversal/update from synthetic function entry counts so it's not exclusive to my changes. I'm looking into this but sending this out in the mean time for review.

[modimo]

@tejohnson
I was looking at the initial implementation and looking at purely the front entry of the SummaryList seems suspect given:

1. There could be multiple entries from linkonce_odr
2. The first entry isn't guaranteed to be a FunctionSummary

Looking around I saw that SafeStack (https://github.com/llvm/llvm-project/commit/47552a614a8c95e1817d83755a4a6a2508da7f8a) took these into account with findCalleeFunctionSummary which to me makes more sense as an implementation. For function attribute propagation, we can also handle linkonce_odr naively by doing a conservative union of the attributes.

More aggressively, propagating based off of the prevailing definition would be more powerful/simpler but there's some nuance shown in the linkonce_functionattrs_comdat.ll test added. If we propagated the prevailing definition g would be marked as norecurse. However, if the local copy of f is inlined into g that attribute would be incorrect and AFAICT currently thinLTO doesn't enforce using the prevailing definition everywhere presumably because it will break code that relies on using the local copy.

I'll definitely measure the compile time cost of this. Wanted to get some feedback on if this path makes sense first.

[modimo]

Clean up logic around bailing out. Separate out thinLTO statistics from general statistics. Run clang-format.

Gathering some stats on Clang release thinlto self-build:
"function-attrs.NumThinLTONoRecurse": 1542018,

Which isn't unexpected given that the vast majority of functions are norecurse. The counting statistics here are a bit skewed because they are done per summary updated and with operating on weak symbols it may make more sense to count once per ValueInfo.

Notably no other statistics shifted with enabling/disabling propagation and a search through LLVM shows only globalopt processInternalGlobal actually uses norecurse ATM. Stats-wise this doesn't fire in either case so it makes sense why codegen doesn't shift.

In that sense, propagating norecurse doesn't seem very valuable. I then tested out nounwind as a better candidate given that currently propagation halts on de-refinable/weak functions when done per-module. Processing in the index, we know every definition and if there's any external definitions meaning propagation can be done confidently through these functions.

Gathering some stats I added (D104161) on a release thinlto -fno-inline build (inlining duplicates landing pads which makes comparisons harder):

thinlto/

"dwarfehprepare.NumFunctionsProcessed": 630099,
"dwarfehprepare.NumCleanupLandingPads": 214916,
"dwarfehprepare.NumNoUnwind": 299357,

thinlto_withpropagation/

"dwarfehprepare.NumFunctionsProcessed": 629976,
"dwarfehprepare.NumCleanupLandingPads": 160635,
"dwarfehprepare.NumNoUnwind": 412014,

Shows a 412014/299357=38% increase in NoUnwind functions and a 160635/214916=25% decrease in the number of cleanup landing pads which is a considerable reduction in EH code.

Measuring performance of Thin Link on Clang self-build I'm seeing a pretty nasty hit of 10s baseline -> 24s with propagation. I'm investigating because as-is this is way too much.

[modimo]

ping @tejohnson/ @mehdi_amini

[tejohnson]

In D36850#2823227, @modimo wrote:

ping @tejohnson/ @mehdi_amini

Sorry for not being responsive. I've been out of office but will be back Monday. I skimmed through your notes, thanks for all the stats, it looks like nounwind is a good direction. Regarding linkonce_odr, I would think you should be able to take the union of their attributes, since they should be interchangeable. There should not be a requirement to use the local copy for linkonce_odr. After propagation, wouldn't their attributes be the same (i.e. regardless of inlining, since the callee attributes should presumably propagate up into the callee)? How much difference would it cause on your statistics and on the compile time to not use the prevailing copy?

[modimo]

In D36850#2827176, @tejohnson wrote:

Sorry for not being responsive. I've been out of office but will be back Monday.

No worries!

I skimmed through your notes, thanks for all the stats, it looks like nounwind is a good direction. Regarding linkonce_odr, I would think you should be able to take the union of their attributes, since they should be interchangeable.

Looking into this more my example in linkonce_functionattrs_comdat.ll is UB given that it's violating language ODR with 2 functionally different definitions. I think under the langref we're safe to propagate off of any single copy of a linkonce_odr however practically speaking taking the union of all of them may be safer and also handle cases where different attributes appear due to de-refinement.

There should not be a requirement to use the local copy for linkonce_odr. After propagation, wouldn't their attributes be the same (i.e. regardless of inlining, since the callee attributes should presumably propagate up into the callee)?

In the original approach without taking the union a different copy could propagate "norecurse" to the caller but the local definition could actually recurse. If this is inlined, we'll get a caller with the "norecurse" attribute incorrectly which in reality does recurse. Taking the conservative union fixes this issue.

[tejohnson]

In D36850#2828461, @modimo wrote:

In D36850#2827176, @tejohnson wrote:

Sorry for not being responsive. I've been out of office but will be back Monday.

No worries!

Thanks again for working on this! Finally had a chance to go through the whole patch.

I skimmed through your notes, thanks for all the stats, it looks like nounwind is a good direction. Regarding linkonce_odr, I would think you should be able to take the union of their attributes, since they should be interchangeable.

Looking into this more my example in linkonce_functionattrs_comdat.ll is UB given that it's violating language ODR with 2 functionally different definitions. I think under the langref we're safe to propagate off of any single copy of a linkonce_odr however practically speaking taking the union of all of them may be safer and also handle cases where different attributes appear due to de-refinement.

There should not be a requirement to use the local copy for linkonce_odr. After propagation, wouldn't their attributes be the same (i.e. regardless of inlining, since the callee attributes should presumably propagate up into the callee)?

In the original approach without taking the union a different copy could propagate "norecurse" to the caller but the local definition could actually recurse. If this is inlined, we'll get a caller with the "norecurse" attribute incorrectly which in reality does recurse. Taking the conservative union fixes this issue.

One possibility is to just look at the copy the linker chose as prevailing. We can pass in the isPrevailing callback (see the call to thinLTOInternalizeAndPromoteInIndex just before your new calls to thinLTOPropagateFunctionAttrs).

I have some other ideas listed in the embedded comments for hopefully making it a bit more efficient, and some other comments/questions.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

104

Might be good to commit this off by default at first, and enable for the new tests. Then it will be easier to do more extensive testing (correctness, compile time, performance), e.g. for our internal apps.

342

In general there needs to be better comments below for the various cases. I see, this looks to be cloned from the StackSafety version, which unfortunately did not undergo a code review before commit and I missed until now... Some questions below that you'll probably need to investigate.

351

We should never have !hasAliasee() here. That should only be true in a couple special cases which don't apply here (in the backends when reading a partial index file emitted for distributed ThinLTO, or when building summaries when reading llvm assembly).

356

If the linkage is local and we have more than one summary for this guid we can probably just quit early - that should be a weird corner case that can be handled conservatively, i.e. by not propagating. Normally we expect that local symbols from different modules will have different guid's and therefore ValueInfos because the guid is computed by prepending the module path. We could have a guid alias if there wasn't any distinguishing path when each file was compiled, but that should be rare and we can just punt.

359

What if the other summaries are seen after this external linkage summary in the list?

Presumably these cases would be when we have a strong def that overrides weak definitions. In either case the external linkage symbol would presumably have been prevailing, and we can probably assert on that fact here if we pass in the isPrevailing callback.

363

For weak and the below available externally and linkonce cases, we presumably could:

• pick the first one if ODR
• pick the prevailing copy in all cases

I think? We can pass in the isPrevailing callback (see the calls to thinLTOInternalizeAndPromoteInIndex just before the calls you added to thinLTOPropagateFunctionAttrs).

373

Can this handling be folded into the above loop so that we don't have to walk the list of summaries again? I.e. a lambda called for each summary before adding to the list of summaries.

Also, I think the whole while loop could be replaced with something like:

if (FunctionSummary *FS = dyn_cast<FunctionSummary>(S->getBaseObject()))
  ResolvedSummaries.push_back(FS);
else
  return {};

See my specific notes below about some of the cases currently being handled.

380

We should never have !hasAliasee() here. That should only be true in a couple special cases which don't apply here (in the backends when reading a partial index file emitted for distributed ThinLTO, or when building summaries when reading llvm assembly).

383

I don't understand what would cause this case.

418

It seems like we would do the same thing here many times for a frequently called function. Can we save some info in a lazily built map and reuse it when already computed? I.e. either the result of getFunctionSummaries or even better a bool of whether any of those summaries might recurse.

430

Can this be folded into the above loop, i.e. where it is currently doing the std::for_each and adding to calleeSummaries? Just update calleesMightRecurse there instead of adding to a new set and walking again here.

436

Instead of adding the new setFFlags and doing the checking first here, perhaps just add an interface to set the NoRecurse flag on S and call it unconditionally? Eventually setters can be added for other flags as needed.

[modimo]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

104

Sure, flipped.

342

Agreed, I've added summarizing comments on what we're doing here.

351

Makes sense, changed.

356

That makes sense, punting on it.

373

I think the original intention from the unreviewed code is that peel through indirection layers of AS->AS->FS. I suspect this isn't too common, I can try adding some diagnostic code to see how many indirections we need as part of the patch.

383

This and the rest of the functions are being re-implemented from top-down.

[modimo]

I'm headed out on vacation until Aug. 1st so sending an update of WIP-ish revision. Will definitely pick this up again when I get back :D

Appreciate the review and the explanations @tejohnson. I did a full refactoring of how we calculate attributes and added a caching layer.

I added WIP nounwind propagation so folks can look it over and try it out, known deficiencies:

1. thinLTO version hasn't been updated for the new field. Will definitely do that (or split it out from NoRecurse as a separate change?)
2. testing needs to be cleaned up for this field

Also:

1. Need to evaluate timetrace to see if the caching strategy is better for build time
2. Add quite a bit more testing to validate how we're handling all the different linkage scenarios

For reference on nounwind propagation now:
Build compiler with this change on top of b16400449fc763fdae2d2ce809ce61c88acb6684 building cd0a1226b50081e86eb75a89d01e8782423971a0

thinlto/

"dwarfehprepare.NumCleanupLandingPadsRemaining": 216754,
"dwarfehprepare.NumNoUnwind": 300957,
"dwarfehprepare.NumUnwind": 332785,

thinlto_withpropagation/

"dwarfehprepare.NumCleanupLandingPadsRemaining": 90401,
"dwarfehprepare.NumNoUnwind": 513744,
"dwarfehprepare.NumUnwind": 119360,

[tejohnson]

Thanks - I know you are still working on this, but I had a few comments so far. I haven't had a chance to test it yet. Unfortunately, the nounwind propagation shouldn't do much on our side as we disable exceptions internally.

clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

39

Do we expect the new noUnwind flag to show up here somewhere?

clang/test/CodeGen/thinlto-distributed-cfi.ll

27

Ditto

llvm/include/llvm/IR/ModuleSummaryIndex.h

592

The capitalization here is off - the LLVM asm parsing expects lower camel case.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

359

Of the below cases, 1 can happen and we should just do something conservative. 2 and 3 should not and we can assert.

378

When does this happen?

410

If for some reason (GUID alias due to local name without paths or other rare case) we get a non FunctionSummary, just early return {} and remove a level of nesting?

413

This can happen for the reasons mentioned in your comment above. No need for an error message, just early return {} for conservative behavior.

426

This should never happen. The linker should already have given a multiply defined symbol error.

448

Won't we still have a copy marked prevailing? Wondering if the weak linkage cases can all be merged.

450

I don't understand this case. If there is no prevailing symbol in the IR for this GUID then presumably it should not have been marked live. Are you seeing this kick in?

458

In all of the cases here, other than the hasNonODR case which I don't understand (yet), we should have a single prevailing FunctionSummary. Can we just cache that, rather than e.g. copying all of its callees?

573

ReadNone and ReadOnly aren't getting propagated yet afaict, so probably add a note to that effect here.

581

Nit: suggest simplifying all of these to something like:
if (FS->fflags().NoRecurse && !F.doesNotRecurse())

F.setDoesNotRecurse();

[modimo]

clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

39

Yeah. I changed the printing logic back to not print anything when no flags are set since quite a few tests depended on this behavior so these tests ultimately remain unchanged. I added a new test here to explicitly check that the flags from propagation show up correctly.

llvm/include/llvm/IR/ModuleSummaryIndex.h

592

Changing back to camel case, also added parsing code for noUnwind.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

378

Good question. I looked further into exactly what triggers this in clang self-build and there are summaries which are AvailableExternally but have no Prevailing copy. I added more about this in the source comments but TL;DR these end up being edge cases that can be ignored.

Changed the logic to go conservative in these cases.

410

Sounds good, removed

448

Yeah, there will still be a copy that's prevailing. Reading through the linkage descriptions again and also those in FunctionImportGlobalProcessing::getLinkage:

1. I think with External/WeakODR/LinkOnceODR once the prevailing is found use that copy

2. For Weak/LinkOnce even with a prevailing copy I don't know if the copy ultimately used will be prevailing. I'm wondering if a native definition could be the victor in which case we just can't propagate off these functions.

WDYT about (2)? For C++ at least these don't seem to really exist and testing with Clang self-build I'm not seeing this kick in anywhere. I added a flag to specifically disable this case so it's easy to test out the differences.

450

Same answer as above:

Good question. I looked further into exactly what triggers this in clang self-build and there are summaries which are AvailableExternally but have no Prevailing copy. I added more about this in the comments but TL;DR these end up being edge cases that can be ignored.

Changed the logic to go conservative in these cases.

458

Good idea, I'll do that. The combined case I suspect will be quite rare in C++ code so the cache only captures the FunctionSummaries and leaves generating a merged callee graph at use time.

[modimo]

Adding more test cases and changed logic around weak linkages

[modimo]

@tejohnson Indirect calls are not captured in FunctionSummaries in CallGraph or in a flag form saying they exist. Also looks like speculative candidates for ICP do make it on the graph. For this analysis we need to bail out on indirect calls so I think the simplest way is to add a flag indicating the presence of them (In FunFlags?). As for the speculative candidates, it's probably not too big of a deal.

In D36850#2940598, @tejohnson wrote:

Thanks - I know you are still working on this, but I had a few comments so far. I haven't had a chance to test it yet. Unfortunately, the nounwind propagation shouldn't do much on our side as we disable exceptions internally.

I've been iterating on this with Clang self-build with exceptions enabled. Once this gets into a good state logically I'll start testing on some of our internal workloads which generally enable exceptions and report back on the findings.

[modimo]

Minor test fixups

[modimo]

Remove llvm/test/ThinLTO/X86/weak_externals.ll from diffs

[modimo]

Checking build timing in release mode Clang self-build. Looking at purely thinlink timing:

Mode	Time (s)
base	2.254
base + propagation	2.556
noinline	8.870
noinline + propagation	10.215

So 13% in base and 15% with noinline which seems reasonable for what it's doing.

[tejohnson]

In D36850#2958594, @modimo wrote:

@tejohnson Indirect calls are not captured in FunctionSummaries in CallGraph or in a flag form saying they exist. Also looks like speculative candidates for ICP do make it on the graph. For this analysis we need to bail out on indirect calls so I think the simplest way is to add a flag indicating the presence of them (In FunFlags?). As for the speculative candidates, it's probably not too big of a deal.

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect. For speculative devirtualization aka ICP, we will still be left with a fallback indirect call, so it would still need to be treated conservatively. The extra edges added for ICP promotion candidates shouldn't be a problem or affect this.

Note that with class hierarchy analysis we can do better for virtual calls, e.g. when -fwhole-program-vtables is enabled for whole program devirtualization and we have the necessary whole program visibility on vtables. We could potentially integrate WPD decision here. Even if we can't find a single devirtualization target, we can compute the set of all possible targets of virtual calls during the thin link and could use that information to basically merge the attributes from all possible targets. But probably best to leave that as a future refinement as it will take some additional work to get that hooked up. We'd still need to be conservative for virtual calls when we don't have the necessary type info (when -fwhole-program-vtables not enabled or we don't have whole program visibility on the vtable defs), or for non-virtual indirect calls.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

384

I'm not sure how this case could be happening as we haven't actually done the importing that would create these new available externally copies yet - that happens in the LTO backends, during the thin link we just add them to import lists.

389

There is no prevailing copy presumably because the prevailing copy is in a native library being linked? I think these cases can be handled conservatively.

414

You can just unconditionally do the getBaseObject call on the GVS without casting to AliasSummary. For non-AliasSummary it will just return itself.

420

I think this checking for virtual calls will only work if -fwhole-program-vtables is enabled for whole program devirtualization or CFI. Otherwise we don't have the type tests that cause this to get populated. This also won't detect non-virtual indirect calls.

448

Since the linker which invokes this should have been passed all objects to link, bitcode and native, it can do symbol resolution across all of them. So if there is an overriding native strong symbol, it should see that and the bitcode resolution would be non-prevailing and all bitcode copies marked dead (in computeDeadSymbols). So I think the weak any and linkonce any case can take the prevailing copy.

[modimo]

In D36850#2964293, @tejohnson wrote:

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect.

That could have an issue where A calls {indirect, B} and A gets propagated upon from B without knowledge that the indirect call exists. Right now I've got a FunFlags hasUnknownCall which marks these as non-propagatable.

For speculative devirtualization aka ICP, we will still be left with a fallback indirect call, so it would still need to be treated conservatively. The extra edges added for ICP promotion candidates shouldn't be a problem or affect this.

Ah good point. I was thinking it may pessimize the propagation because of having to process all of these edges this is a no-go because of the fallback.

Note that with class hierarchy analysis we can do better for virtual calls, e.g. when -fwhole-program-vtables is enabled for whole program devirtualization and we have the necessary whole program visibility on vtables. We could potentially integrate WPD decision here. Even if we can't find a single devirtualization target, we can compute the set of all possible targets of virtual calls during the thin link and could use that information to basically merge the attributes from all possible targets. But probably best to leave that as a future refinement as it will take some additional work to get that hooked up. We'd still need to be conservative for virtual calls when we don't have the necessary type info (when -fwhole-program-vtables not enabled or we don't have whole program visibility on the vtable defs), or for non-virtual indirect calls.

Agreed, it's an engineering problem more than anything. I ran an optimistic build (same revisions as before, release + noinline) where indirect and virtual calls were assumed to always propagate (thinlto_prop_optimistic) and the effect in Clang self-build at least is not too large:

thinlto_base/

"dwarfehprepare.NumCleanupLandingPadsRemaining": 217515,
"dwarfehprepare.NumNoUnwind": 299126,
"dwarfehprepare.NumUnwind": 332785,

thinlto_prop/

"dwarfehprepare.NumCleanupLandingPadsRemaining": 158372,
"dwarfehprepare.NumNoUnwind": 420918,
"dwarfehprepare.NumUnwind": 210870,

thinlto_prop_optimistic/

"dwarfehprepare.NumCleanupLandingPadsRemaining": 154958,
"dwarfehprepare.NumNoUnwind": 425893,
"dwarfehprepare.NumUnwind": 205889,

(425893-420918)/(420918-299126) = 4% boost over being conservative and correct. This may change in real workloads though so I added a thinlto-funcattrs-optimistic-indirect flag for easy measurement.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

384

I added the test funcattrs-prop-exported-internal.ll that demonstrates this. The summary has its internal linkage converted to external in thinLTOResolvePrevailingInIndex then converted to AvailableExternally in thinLTOResolvePrevailingGUID. Currently being handled conservatively since a prevailing copy doesn't exist.

389

Yeah the prevailing copy is in the native binary.

This is a C++ specific feature which has ODR and these are already being propagated/inlined from in pre-link. The current thinlink propagation implementation is conservative because a prevailing copy doesn't exist. Currently being handled conservatively since a prevailing copy doesn't exist.

420

I see. I added hasUnknownCall as an explicit flag for all indirect calls that should capture both cases.

448

That makes it much easier and everything folds into the prevailing case! Changed and added a test for it.

[modimo]

Use prevailing for linkonce/weak. Add hasUnknownCall to model virtual and indirect calls

[modimo]

Check for CachedAttributes count in map rather than value so conservative results are not re-calculated

[modimo]

gentle ping @tejohnson

[tejohnson]

In D36850#2968536, @modimo wrote:

In D36850#2964293, @tejohnson wrote:

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect.

That could have an issue where A calls {indirect, B} and A gets propagated upon from B without knowledge that the indirect call exists. Right now I've got a FunFlags hasUnknownCall which marks these as non-propagatable.

Ah, because there isn't a conservative setting of the flag...which raises a larger issue (but maybe I am completely missing something) - how do we distinguish between the NoUnwind summary flag not being set because we don't know yet (in which case we want the propagation from callees), vs because it cannot be NoUnwind (because there is a throw in the function)? Do we need to have a second flag indicating that a function contains a mayThrow instruction (other than calls, which are being handled by the propagation)?

[modimo]

In D36850#2990771, @tejohnson wrote:

In D36850#2968536, @modimo wrote:

In D36850#2964293, @tejohnson wrote:

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect.

That could have an issue where A calls {indirect, B} and A gets propagated upon from B without knowledge that the indirect call exists. Right now I've got a FunFlags hasUnknownCall which marks these as non-propagatable.

Ah, because there isn't a conservative setting of the flag...which raises a larger issue (but maybe I am completely missing something) - how do we distinguish between the NoUnwind summary flag not being set because we don't know yet (in which case we want the propagation from callees), vs because it cannot be NoUnwind (because there is a throw in the function)? Do we need to have a second flag indicating that a function contains a mayThrow instruction (other than calls, which are being handled by the propagation)?

Only call instructions can throw (what ultimately performs the throw operation is an opaque call to __cxa_throw()) which simplifies the problem. If all calls are known, we only need to examine the callees for accurate propagation.

[tejohnson]

In D36850#2990847, @modimo wrote:

In D36850#2990771, @tejohnson wrote:

In D36850#2968536, @modimo wrote:

In D36850#2964293, @tejohnson wrote:

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect.

That could have an issue where A calls {indirect, B} and A gets propagated upon from B without knowledge that the indirect call exists. Right now I've got a FunFlags hasUnknownCall which marks these as non-propagatable.

Ah, because there isn't a conservative setting of the flag...which raises a larger issue (but maybe I am completely missing something) - how do we distinguish between the NoUnwind summary flag not being set because we don't know yet (in which case we want the propagation from callees), vs because it cannot be NoUnwind (because there is a throw in the function)? Do we need to have a second flag indicating that a function contains a mayThrow instruction (other than calls, which are being handled by the propagation)?

Only call instructions can throw (what ultimately performs the throw operation is an opaque call to __cxa_throw()) which simplifies the problem. If all calls are known, we only need to examine the callees for accurate propagation.

What about the other instruction checks done in Instruction::mayThrow i.e. http://llvm-cs.pcc.me.uk/lib/IR/Instruction.cpp#592?

[modimo]

In D36850#2990907, @tejohnson wrote:

In D36850#2990847, @modimo wrote:

In D36850#2990771, @tejohnson wrote:

In D36850#2968536, @modimo wrote:

In D36850#2964293, @tejohnson wrote:

Good point on indirect calls. Rather than add a bit to the summary, can the flags just be set conservatively in any function containing an indirect call when we build the summaries initially? I think that would get the same effect.

That could have an issue where A calls {indirect, B} and A gets propagated upon from B without knowledge that the indirect call exists. Right now I've got a FunFlags hasUnknownCall which marks these as non-propagatable.

Ah, because there isn't a conservative setting of the flag...which raises a larger issue (but maybe I am completely missing something) - how do we distinguish between the NoUnwind summary flag not being set because we don't know yet (in which case we want the propagation from callees), vs because it cannot be NoUnwind (because there is a throw in the function)? Do we need to have a second flag indicating that a function contains a mayThrow instruction (other than calls, which are being handled by the propagation)?

Only call instructions can throw (what ultimately performs the throw operation is an opaque call to __cxa_throw()) which simplifies the problem. If all calls are known, we only need to examine the callees for accurate propagation.

What about the other instruction checks done in Instruction::mayThrow i.e. http://llvm-cs.pcc.me.uk/lib/IR/Instruction.cpp#592?

Good point! CleanupReturnInst and CatchSwitchInst are windows SEH specific representations for asynchronous exceptions but definitely should be covered for correctness. For ResumeInst it's the "return" of a landing pad and in order for a landing pad to be reachable AFAIK an invoke must exist so is captured by the call graph. I'll add a scan for Instruction::mayThrow in summary building. Having a mayThrow flag or making NoUnwind a tri-state flag in the summary makes sense to me to capture this case.

As a side note to why there's a check for ResumeInst at all: an invoke instructions actually never has "mayThrow" set. I haven't delved too deep but this could be changed to check for invokes instead since a dead landing pad at attribute inference time can lead to pessimization of NoUnwind in cases I've seen (alternatively, making sure CFG opts run before this to make sure this is pruned away).

[modimo]

Add mayThrow flag

[tejohnson]

Ok thanks. I need to go through the propagation code and tests again more closely now, but one question/suggestion below in the meantime.

llvm/include/llvm/IR/ModuleSummaryIndex.h

580

Now that we have MayThrow, can we avoid a separate hasUnknownCall bool and just conservatively set MayThrow true in that case?

[modimo]

In D36850#3005254, @tejohnson wrote:

Ok thanks. I need to go through the propagation code and tests again more closely now, but one question/suggestion below in the meantime.

Thanks!

llvm/include/llvm/IR/ModuleSummaryIndex.h

580

hasUnknownCall is used for norecurse and other future flags as well to stop propagation.

[tejohnson]

llvm/include/llvm/IR/ModuleSummaryIndex.h

580

Ah that makes sense.

[tejohnson]

Thanks for your patience, finally had a chance to go through everything much more carefully. Looks good, mostly a bunch of small or nitpicky final suggestions. The main comment/question of significance relates to where hasUnknownCall is being set currently.

Patch title and summary need an update.

clang/test/CodeGen/thinlto-funcattr-prop.ll

15

This is checking the summary generated by opt, not the result of the llvm-lto2 run.

llvm/include/llvm/IR/ModuleSummaryIndex.h

575

No Unwind needs a comment. And probably a note that it will be updated by function attr propagation. Depends on how we want to handle inline asm calls and other cases that currently set this true below (see my comment there).

580

nit, maybe change this to hasIndirectCall which I think is more specific?

llvm/include/llvm/LTO/LTO.h

26

Is this needed?

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

374

Should this be moved below the following checks for inline asm and direct calls? (Not sure what the direct calls case is given that we handle direct calls to "known functions" above though).

If it should stay where it is and treat the below cases as unknown, probably should add tests for them.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

342

Suggest renaming calculateDefinitiveAttributes and CachedAttributes to something like calculatePrevailingSummary and CachedPrevailingSummary which are more accurate now.

484

You've already set InferredFlags.NoUnwind to false above this loop in the case where MayThrow was set on the CallerSummary.

499

I think you can remove this and the below setNoUnwind() call on CachedAttributes[V] since presumably this points to one of the function summaries we update in the below loop.

531

nit: suggest "Insert propagated function attributes from the Index ..."

537

Consider consolidating this function with thinLTOResolvePrevailingInModule, to reduce the number of walks of the module and lookups into the DefinedGlobals map.

llvm/test/ThinLTO/X86/funcattrs-prop-indirect.ll

4

Have a second version that tests with -thinlto-funcattrs-optimistic-indirect? I don't see a test for that option anywhere. Or maybe just remove that option - is it really needed?

10

Perhaps this CHECK-NOT should just look for "Function Attrs:" as you do in some other tests below, in case some other attr is ever added to the IR that isn't related to this propagation, which could allow the CHECK-NOT to succeed for the wrong reasons?

llvm/test/ThinLTO/X86/funcattrs-prop.ll

9

Since linkonce and weak are interposable, it isn't really correct to say that individual callers may optimize using different copies (we try to prevent this in the compiler since the are interposable).

30

s/recursing/throwing/ on these 2 comments?

64

Suggest putting comments above this one and call_weak_may_unwind below to indicate why one gets the nounwind and the other doesn't (i.e. that the thin link command above selects as prevailing the nounwind version of linkonce_may_unwind from b.ll and the may throw version of weak_may_unwind from c.ll)

77

For clarity on what this is actually testing, suggest renaming these as ATTR_NOUNWIND and ATTR_MAYTHROW, or something like that (they are both norecurse, so the current name is a little misleading as to what is being checked).

[tejohnson]

llvm/include/llvm/IR/ModuleSummaryIndex.h

575

Woops, the second sentence here "Depends on how we want to handle inline asm calls and other cases that currently set this true below (see my comment there)." I meant to add to the comment further below about the name of hasUnknownCall.

[modimo]

Address feedback, rename funcattrs-prop-indirect.ll to funcattrs-prop-unknown.ll

clang/test/CodeGen/thinlto-funcattr-prop.ll

15

Fixed.

llvm/include/llvm/IR/ModuleSummaryIndex.h

580

My thinking is that the flag is a catch-all for blocking propagation and could conceivably be set for other reasons. It also matches the existing usage in FunctionAttrs.cpp for local propagation which also sets this for functions that are OptNone.

llvm/include/llvm/LTO/LTO.h

26

Yeah, thinLTOPropagateFunctionAttrs resides in FunctionAttrs.h and runThinLTO calls it to propagate.

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

374

Any call that isn't emitted to the summary CallGraphEdges is a hole in propagation knowledge.

Direct calls case is from https://reviews.llvm.org/D40056 which is handling:

; Test calls that aren't handled either as direct or indirect.
call void select (i1 icmp eq (i32* @global, i32* null), void ()* @f, void ()* @g)()

Neat that select can be consolidated into a call, though I wonder if it should be allowed given it could be canonicalized to be another IR instruction above it and maybe eliminate this edge case.

Tangent aside, since in all these cases the call isn't part of the static callgraph HasUnknownCall needs to be set for correctness. Tests added in funcattrs-prop-unknown.ll (replacing funcattrs-prop-indirect.ll since we're handling more than just indirect here).

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

484

Good catch, this case should be querying CalleeSummary MayThrow.

499

Makes sense, removed. I like keeping the stats/debug tracking around though.

537

Good idea, merged and renamed thinLTOResolvePrevailingInModule to thinLTOFinalizeInModule

llvm/test/ThinLTO/X86/funcattrs-prop-indirect.ll

4

Good point, option removed.

llvm/test/ThinLTO/X86/funcattrs-prop.ll

9

True, this comment is a left-over from the first interpretation of linkage models which have since been fixed with your help :). I'll update this to reflect that only prevailing matters here.

[tejohnson]

Few follow ups below.

clang/test/CodeGen/thinlto-funcattr-prop.ll

17

I believe this corresponds to call_extern - why aren't we getting noRecurse and noUnwind propagated here?

(also, suggest adding a comment above each of these summaries as to what function name they correspond to)

llvm/include/llvm/Transforms/IPO/FunctionImport.h

224 ↗

(On Diff #374431)

Suggest either removing default since you are always passing this argument, or default it to true and stop passing it in the places where it is true (since generally we want this to be true except in a few ThinLTOCodeGenerator.cpp locations that are testing specific things that don't involve propagation). Some preference for the former option (removing default), to make sure any new callers that get added think through the appropriate value.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

485

Please make sure one of the may throw propagation tests would fail without this fix (i.e. when it was checking the caller's maythrow setting).

llvm/lib/Transforms/IPO/FunctionImport.cpp

1110 ↗

(On Diff #374431)

Can this be merged with updateLinkage so we only do the DefinedGlobals lookup once per symbol?

llvm/test/ThinLTO/X86/funcattrs-prop.ll

7

Nit, line length

[modimo]

Address follow-ups

clang/test/CodeGen/thinlto-funcattr-prop.ll

17

Tracing through llvm-lto2 the index is written out by CombinedIndexHook before the rest of thinlink including attribute propagation takes place. The attributes do end up successfully getting propagated, I'll add a check for that in the *1.promote.bc which shows the outcome of the attributes being propagated.

Good idea, added the function name that correspond to each summary.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

485

Thinking more on why this didn't manifest strange behavior: because of the BU order of call-graph traversal any callee that has mayThrow will have its inferred noUnwind set to false above. Checking again in the caller is redundant because the noUnwind property of the callee will be determined by its value of noUnwind only. I think removing this check completely makes sense.

I can think of a scenario where there are mayThrow instructions but the function is still marked noUnwind (noexcept function with a throw in it) but in that case it is safe to propagate upwards because any exception will fail to escape this callee and so checking mayThrow would actually be a pessimization. I added a case in funcattrs-prop-maythrow.ll to illustrate this.

llvm/lib/Transforms/IPO/FunctionImport.cpp

1110 ↗

(On Diff #374431)

Sure, merged.

[tejohnson]

lgtm (one minor comment issue noted below). Thanks!

clang/test/CodeGen/thinlto-funcattr-prop.ll

17

Incomplete sentence, seems to be missing the rest of the explanation about when it is written.

[modimo]

Complete explanation in thinlto-funcattr-prop.ll, also fix up diff to contain all changes.

clang/test/CodeGen/thinlto-funcattr-prop.ll

17

Nice catch, sentence is now complete.

[modimo]

Thanks for the thorough review @tejohnson! I'll do additional validation on FB code that uses exceptions and if that all looks good I'll send up a change to turn this default on with the findings.

clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

 ; REQUIRES: x86-registered-target
 
 ; Backend test for distribute ThinLTO with CFI.
 ; It additionally enables -fwhole-program-vtables to get more information in
 ; TYPE_IDs of GLOBALVAL_SUMMARY_BLOCK.
 
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
 ; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39436.
-; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
+; RUN: llvm-lto2 run -thinlto-distributed-indexes -disable-thinlto-funcattrs=0 %t.o \
 ; RUN:   -whole-program-visibility \
 ; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t2.index \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t.o,_ZN1C1fEi,p \
 ; RUN:   -r=%t.o,_ZTV1B, \

 ; CHECK-LABEL: <STRTAB_BLOCK
 ; CHECK: blob data = '_ZTS1A_ZN1A1nEi'
 ; CHECK-LABEL: </STRTAB_BLOCK
 
 ; RUN: llvm-dis %t.o.thinlto.bc -o - | FileCheck %s --check-prefix=CHECK-DIS
 ; Round trip it through llvm-as
 ; RUN: llvm-dis %t.o.thinlto.bc -o - | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK-DIS
 ; CHECK-DIS: ^0 = module: (path: "{{.*}}thinlto-distributed-cfi-devirt.ll.tmp.o", hash: ({{.*}}, {{.*}}, {{.*}}, {{.*}}, {{.*}}))
-; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, typeIdInfo: (typeTests: (^2), typeCheckedLoadVCalls: (vFuncId: (^2, offset: 8), vFuncId: (^2, offset: 0))))))
+; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 18, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTests: (^2), typeCheckedLoadVCalls: (vFuncId: (^2, offset: 8), vFuncId: (^2, offset: 0))))))
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: allOnes, sizeM1BitWidth: 7), wpdResolutions: ((offset: 0, wpdRes: (kind: branchFunnel)), (offset: 8, wpdRes: (kind: singleImpl, singleImplName: "_ZN1A1nEi"))))) ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
 ; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc -O2 -Rpass=wholeprogramdevirt \
 ; RUN:   -emit-llvm -o - -x ir %t.o 2>&1 | FileCheck %s --check-prefixes=CHECK-IR --check-prefixes=REMARKS
 
 ; Check that the devirtualization is suppressed via -wholeprogramdevirt-skip
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu -mllvm -wholeprogramdevirt-skip=_ZN1A1nEi \

clang/test/CodeGen/thinlto-distributed-cfi.ll

 ; REQUIRES: x86-registered-target
 
 ; Backend test for distribute ThinLTO with CFI.
 
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
-; RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
+; RUN: llvm-lto2 run -thinlto-distributed-indexes -disable-thinlto-funcattrs=0 %t.o \
 ; RUN:   -o %t2.index \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZTV1B, \
 ; RUN:   -r=%t.o,_ZN1B1fEi, \
 ; RUN:   -r=%t.o,_ZTV1B,px
 
 ; Check that typeids are in the index.
 ; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s
 ; CHECK-LABEL: <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK: <TYPE_ID op0=0 op1=6 op2=3 op3=0 op4=0 op5=0 op6=0 op7=0/>
 ; CHECK-LABEL: </GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-LABEL: <STRTAB_BLOCK
 ; CHECK: blob data = '_ZTS1A'
 ; CHECK-LABEL: </STRTAB_BLOCK
 
 ; RUN: llvm-dis %t.o.thinlto.bc -o - | FileCheck %s --check-prefix=CHECK-DIS
 ; Round trip it through llvm-as
 ; RUN: llvm-dis %t.o.thinlto.bc -o - | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK-DIS
 ; CHECK-DIS: ^0 = module: (path: "{{.*}}thinlto-distributed-cfi.ll.tmp.o", hash: ({{.*}}, {{.*}}, {{.*}}, {{.*}}, {{.*}}))
-; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 7, typeIdInfo: (typeTests: (^2)))))
+; CHECK-DIS: ^1 = gv: (guid: 8346051122425466633, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 7, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, hasUnknownCall: 0), typeIdInfo: (typeTests: (^2)))))
 ; CHECK-DIS: ^2 = typeid: (name: "_ZTS1A", summary: (typeTestRes: (kind: single, sizeM1BitWidth: 0))) ; guid = 7004155349499253778
 
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \
 ; RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
 ; RUN:   -emit-llvm -o - -x ir %t.o | FileCheck %s --check-prefixes=CHECK-IR
 
 ; Ensure that backend does not fail generating native code.
 ; RUN: %clang_cc1 -triple x86_64-grtev4-linux-gnu \

clang/test/CodeGen/thinlto-funcattr-prop.ll

+; REQUIRES: x86-registered-target
+
+; Test that FunctionAttr Propagation is generating correct summaries
+
+; RUN: split-file %s %t
+; RUN: opt -module-summary %t/a.ll -o %t/a.bc
+; RUN: opt -module-summary %t/b.ll -o %t/b.bc
+
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t/a.bc %t/b.bc -o %t1 -save-temps \
+; RUN:   -r=%t/a.bc,call_extern,plx \
+; RUN:   -r=%t/a.bc,extern, \
+; RUN:   -r=%t/b.bc,extern,p
+
+; RUN: llvm-dis %t/b.bc -o - | FileCheck %s
+
+; CHECK: ^0 = module: (path: "{{.*}}b.bc", hash: ({{.*}}, {{.*}}, {{.*}}, {{.*}}, {{.*}}))
+; CHECK: ^1 = gv: (name: "extern", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 1, hasUnknownCall: 0)))) ; guid = 14959766916849974397
+; CHECK: ^2 = blockcount: 1
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @extern()
+
+define void @call_extern() {
+    call void @extern()
+    ret void
+}
+
+;--- b.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+attributes #0 = { nounwind norecurse }
+
+define void @extern() #0 {
+  ret void
+}
 No newline at end of file

llvm/include/llvm/AsmParser/LLToken.h

enum Kind {
   kw_insts,
   kw_funcFlags,
   kw_readNone,
   kw_readOnly,
   kw_noRecurse,
   kw_returnDoesNotAlias,
   kw_noInline,
   kw_alwaysInline,
+  kw_noUnwind,
+  kw_hasUnknownCall,
   kw_calls,
   kw_callee,
   kw_params,
   kw_param,
   kw_hotness,
   kw_unknown,
   kw_hot,
   kw_critical,

llvm/include/llvm/IR/GlobalValue.h

public:
   }
 
   static bool isExternalLinkage(LinkageTypes Linkage) {
     return Linkage == ExternalLinkage;
   }
   static bool isAvailableExternallyLinkage(LinkageTypes Linkage) {
     return Linkage == AvailableExternallyLinkage;
   }
+  static bool isLinkOnceAnyLinkage(LinkageTypes Linkage) {
+    return Linkage == LinkOnceAnyLinkage;
+  }
   static bool isLinkOnceODRLinkage(LinkageTypes Linkage) {
     return Linkage == LinkOnceODRLinkage;
   }
   static bool isLinkOnceLinkage(LinkageTypes Linkage) {
-    return Linkage == LinkOnceAnyLinkage || Linkage == LinkOnceODRLinkage;
+    return isLinkOnceAnyLinkage(Linkage) || isLinkOnceODRLinkage(Linkage);
   }
   static bool isWeakAnyLinkage(LinkageTypes Linkage) {
     return Linkage == WeakAnyLinkage;
   }
   static bool isWeakODRLinkage(LinkageTypes Linkage) {
     return Linkage == WeakODRLinkage;
   }
   static bool isWeakLinkage(LinkageTypes Linkage) {
public:
   bool isInterposable() const;
   bool canBenefitFromLocalAlias() const;
 
   bool hasExternalLinkage() const { return isExternalLinkage(getLinkage()); }
   bool hasAvailableExternallyLinkage() const {
     return isAvailableExternallyLinkage(getLinkage());
   }
   bool hasLinkOnceLinkage() const { return isLinkOnceLinkage(getLinkage()); }
+  bool hasLinkOnceAnyLinkage() const {
+    return isLinkOnceAnyLinkage(getLinkage());
+  }
   bool hasLinkOnceODRLinkage() const {
     return isLinkOnceODRLinkage(getLinkage());
   }
   bool hasWeakLinkage() const { return isWeakLinkage(getLinkage()); }
   bool hasWeakAnyLinkage() const { return isWeakAnyLinkage(getLinkage()); }
   bool hasWeakODRLinkage() const { return isWeakODRLinkage(getLinkage()); }
   bool hasAppendingLinkage() const { return isAppendingLinkage(getLinkage()); }
   bool hasInternalLinkage() const { return isInternalLinkage(getLinkage()); }

llvm/include/llvm/IR/ModuleSummaryIndex.h

struct FFlags {
     unsigned ReadOnly : 1;
     unsigned NoRecurse : 1;
     unsigned ReturnDoesNotAlias : 1;
 
     // Indicate if the global value cannot be inlined.
     unsigned NoInline : 1;
     // Indicate if function should be always inlined.
     unsigned AlwaysInline : 1;
+    unsigned NoUnwind : 1;
+
+    // If there are calls to unknown targets (e.g. indirect)
+    unsigned hasUnknownCall : 1;
+
+    FFlags &operator&=(const FFlags &RHS) {
+      this->ReadNone &= RHS.ReadNone;
+      this->ReadOnly &= RHS.ReadOnly;
+      this->NoRecurse &= RHS.NoRecurse;
+      this->ReturnDoesNotAlias &= RHS.ReturnDoesNotAlias;
+      this->NoInline &= RHS.NoInline;
+      this->AlwaysInline &= RHS.AlwaysInline;
+      this->NoUnwind &= RHS.NoUnwind;
+      this->hasUnknownCall &= RHS.hasUnknownCall;
+      return *this;
+    }
+
+    bool anyFlagSet() {
+      return this->ReadNone | this->ReadOnly | this->NoRecurse |
+             this->ReturnDoesNotAlias | this->NoInline | this->AlwaysInline |
+             this->NoUnwind | this->hasUnknownCall;
+    }
+
+    operator std::string() {
+      std::string Output;
+      raw_string_ostream OS(Output);
+      OS << "funcFlags: (";
+      OS << "readNone: " << this->ReadNone;
+      OS << ", readOnly: " << this->ReadOnly;
+      OS << ", noRecurse: " << this->NoRecurse;
+      OS << ", returnDoesNotAlias: " << this->ReturnDoesNotAlias;
+      OS << ", noInline: " << this->NoInline;
+      OS << ", alwaysInline: " << this->AlwaysInline;
+      OS << ", noUnwind: " << this->NoUnwind;
+      OS << ", hasUnknownCall: " << this->hasUnknownCall;
+      OS << ")";
+      return OS.str();
+    }
   };
 
   /// Describes the uses of a parameter by the function.
   struct ParamAccess {
     static constexpr uint32_t RangeWidth = 64;
 
     /// Describes the use of a value in a call instruction, specifying the
     /// call's target, the value's parameter number, and the possible range of
public:
   /// Check if this is a function summary.
   static bool classof(const GlobalValueSummary *GVS) {
     return GVS->getSummaryKind() == FunctionKind;
   }
 
   /// Get function summary flags.
   FFlags fflags() const { return FunFlags; }
 
+  void setNoRecurse() { FunFlags.NoRecurse = true; }
+
+  void setNoUnwind() { FunFlags.NoUnwind = true; }
+
   /// Get the instruction count recorded for this function.
   unsigned instCount() const { return InstCount; }
 
   /// Get the synthetic entry count for this function.
   uint64_t entryCount() const { return EntryCount; }
 
   /// Set the synthetic entry count for this function.
   void setEntryCount(uint64_t EC) { EntryCount = EC; }

llvm/include/llvm/LTO/LTO.h

 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/LTO/Config.h"
 #include "llvm/Object/IRSymtab.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/thread.h"
+#include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 
 namespace llvm {
 
 class Error;
 class IRMover;
 class LLVMContext;
 class MemoryBufferRef;

llvm/include/llvm/Transforms/IPO/FunctionAttrs.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_IPO_FUNCTIONATTRS_H
 #define LLVM_TRANSFORMS_IPO_FUNCTIONATTRS_H
 
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/LazyCallGraph.h"
+#include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
 class AAResults;
 class Function;
 class Module;
 class Pass;
 
 /// The three kinds of memory access relevant to 'readonly' and
 /// 'readnone' attributes.
 enum MemoryAccessKind {
   MAK_ReadNone = 0,
   MAK_ReadOnly = 1,
   MAK_MayWrite = 2,
   MAK_WriteOnly = 3
 };
 
 /// Returns the memory access properties of this copy of the function.
 MemoryAccessKind computeFunctionBodyMemoryAccess(Function &F, AAResults &AAR);
 
+/// Propagate function attributes for function summaries along the index's
+/// callgraph during thinlink
+bool thinLTOPropagateFunctionAttrs(
+    ModuleSummaryIndex &Index,
+    function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+        isPrevailing);
+
+/// Inserts the FunctionAttr flags from the Index into \p TheModule.
+void thinLTOInsertFunctionAttrsForModule(Module &TheModule,
+                                         const GVSummaryMapTy &DefinedGlobals);
+
 /// Computes function attributes in post-order over the call graph.
 ///
 /// By operating in post-order, this pass computes precise attributes for
 /// called functions prior to processsing their callers. This "bottom-up"
 /// approach allows powerful interprocedural inference of function attributes
 /// like memory access patterns, etc. It can discover functions that do not
 /// access memory, or only read memory, and give them the readnone/readonly
 /// attribute. It also discovers function arguments that are not captured by

llvm/lib/Analysis/ModuleSummaryAnalysis.cpp

static void computeFunctionSummary(
   // Add personality function, prefix data and prologue data to function's ref
   // list.
   findRefEdges(Index, &F, RefEdges, Visited);
   std::vector<const Instruction *> NonVolatileLoads;
   std::vector<const Instruction *> NonVolatileStores;
 
   bool HasInlineAsmMaybeReferencingInternal = false;
   bool HasIndirBranchToBlockAddress = false;
+  bool HasUnknownCall = false;
   for (const BasicBlock &BB : F) {
     // We don't allow inlining of function with indirect branch to blockaddress.
     // If the blockaddress escapes the function, e.g., via a global variable,
     // inlining may lead to an invalid cross-function reference. So we shouldn't
     // import such function either.
     if (BB.hasAddressTaken()) {
       for (User *U : BlockAddress::get(const_cast<BasicBlock *>(&BB))->users())
         if (!isa<CallBrInst>(*U)) {
for (const Instruction &I : BB) {
         // Add the relative block frequency to CalleeInfo if there is no profile
         // information.
         if (BFI != nullptr && Hotness == CalleeInfo::HotnessType::Unknown) {
           uint64_t BBFreq = BFI->getBlockFreq(&BB).getFrequency();
           uint64_t EntryFreq = BFI->getEntryFreq();
           ValueInfo.updateRelBlockFreq(BBFreq, EntryFreq);
         }
       } else {
+        HasUnknownCall = true;
         // Skip inline assembly calls.
         if (CI && CI->isInlineAsm())
           continue;
         // Skip direct calls.
         if (!CalledValue || isa<Constant>(CalledValue))
           continue;
 
         // Check if the instruction has a callees metadata. If so, add callees
GlobalValueSummary::GVFlags Flags(
       F.hasLinkOnceODRLinkage() && F.hasGlobalUnnamedAddr());
   FunctionSummary::FFlags FunFlags{
       F.hasFnAttribute(Attribute::ReadNone),
       F.hasFnAttribute(Attribute::ReadOnly),
       F.hasFnAttribute(Attribute::NoRecurse), F.returnDoesNotAlias(),
       // FIXME: refactor this to use the same code that inliner is using.
       // Don't try to import functions with noinline attribute.
       F.getAttributes().hasFnAttr(Attribute::NoInline),
-      F.hasFnAttribute(Attribute::AlwaysInline)};
+      F.hasFnAttribute(Attribute::AlwaysInline),
+      F.hasFnAttribute(Attribute::NoUnwind),
+      HasUnknownCall};
   std::vector<FunctionSummary::ParamAccess> ParamAccesses;
   if (auto *SSI = GetSSICallback(F))
     ParamAccesses = SSI->getParamAccesses(Index);
   auto FuncSummary = std::make_unique<FunctionSummary>(
       Flags, NumInsts, FunFlags, /*EntryCount=*/0, std::move(Refs),
       CallGraphEdges.takeVector(), TypeTests.takeVector(),
       TypeTestAssumeVCalls.takeVector(), TypeCheckedLoadVCalls.takeVector(),
       TypeTestAssumeConstVCalls.takeVector(),
ModuleSymbolTable::CollectAsmSymbols(
                 std::make_unique<FunctionSummary>(
                     GVFlags, /*InstCount=*/0,
                     FunctionSummary::FFlags{
                         F->hasFnAttribute(Attribute::ReadNone),
                         F->hasFnAttribute(Attribute::ReadOnly),
                         F->hasFnAttribute(Attribute::NoRecurse),
                         F->returnDoesNotAlias(),
                         /* NoInline = */ false,
-                        F->hasFnAttribute(Attribute::AlwaysInline)},
+                        F->hasFnAttribute(Attribute::AlwaysInline),
+                        F->hasFnAttribute(Attribute::NoUnwind),
+                        /* HasUnknownCall */ true},
                     /*EntryCount=*/0, ArrayRef<ValueInfo>{},
                     ArrayRef<FunctionSummary::EdgeTy>{},
                     ArrayRef<GlobalValue::GUID>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::ConstVCall>{},
                     ArrayRef<FunctionSummary::ConstVCall>{},
                     ArrayRef<FunctionSummary::ParamAccess>{});

llvm/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(insts);
   KEYWORD(funcFlags);
   KEYWORD(readNone);
   KEYWORD(readOnly);
   KEYWORD(noRecurse);
   KEYWORD(returnDoesNotAlias);
   KEYWORD(noInline);
   KEYWORD(alwaysInline);
+  KEYWORD(noUnwind);
+  KEYWORD(hasUnknownCall);
   KEYWORD(calls);
   KEYWORD(callee);
   KEYWORD(params);
   KEYWORD(param);
   KEYWORD(hotness);
   KEYWORD(unknown);
   KEYWORD(hot);
   KEYWORD(critical);

llvm/lib/AsmParser/LLParser.cpp

 }
 
 /// OptionalFFlags
 ///   := 'funcFlags' ':' '(' ['readNone' ':' Flag]?
 ///        [',' 'readOnly' ':' Flag]? [',' 'noRecurse' ':' Flag]?
 ///        [',' 'returnDoesNotAlias' ':' Flag]? ')'
 ///        [',' 'noInline' ':' Flag]? ')'
 ///        [',' 'alwaysInline' ':' Flag]? ')'
+///        [',' 'noUnwind' ':' Flag]? ')'
+///        [',' 'hasUnknownCall' ':' Flag]? ')'
 
 bool LLParser::parseOptionalFFlags(FunctionSummary::FFlags &FFlags) {
   assert(Lex.getKind() == lltok::kw_funcFlags);
   Lex.Lex();
 
   if (parseToken(lltok::colon, "expected ':' in funcFlags") |
       parseToken(lltok::lparen, "expected '(' in funcFlags"))
     return true;
case lltok::kw_noInline:
       FFlags.NoInline = Val;
       break;
     case lltok::kw_alwaysInline:
       Lex.Lex();
       if (parseToken(lltok::colon, "expected ':'") || parseFlag(Val))
         return true;
       FFlags.AlwaysInline = Val;
       break;
+    case lltok::kw_noUnwind:
+      Lex.Lex();
+      if (parseToken(lltok::colon, "expected ':'") || parseFlag(Val))
+        return true;
+      FFlags.NoUnwind = Val;
+      break;
+    case lltok::kw_hasUnknownCall:
+      Lex.Lex();
+      if (parseToken(lltok::colon, "expected ':'") || parseFlag(Val))
+        return true;
+      FFlags.hasUnknownCall = Val;
+      break;
     default:
       return error(Lex.getLoc(), "expected function flag type");
     }
   } while (EatIfPresent(lltok::comma));
 
   if (parseToken(lltok::rparen, "expected ')' in funcFlags"))
     return true;
 

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

 static FunctionSummary::FFlags getDecodedFFlags(uint64_t RawFlags) {
   FunctionSummary::FFlags Flags;
   Flags.ReadNone = RawFlags & 0x1;
   Flags.ReadOnly = (RawFlags >> 1) & 0x1;
   Flags.NoRecurse = (RawFlags >> 2) & 0x1;
   Flags.ReturnDoesNotAlias = (RawFlags >> 3) & 0x1;
   Flags.NoInline = (RawFlags >> 4) & 0x1;
   Flags.AlwaysInline = (RawFlags >> 5) & 0x1;
+  Flags.NoUnwind = (RawFlags >> 6) & 0x1;
+  Flags.hasUnknownCall = (RawFlags >> 7) & 0x1;
   return Flags;
 }
 
 // Decode the flags for GlobalValue in the summary. The bits for each attribute:
 //
 // linkage: [0,4), notEligibleToImport: 4, live: 5, local: 6, canAutoHide: 7,
 // visibility: [8, 10).
 static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

 static uint64_t getEncodedFFlags(FunctionSummary::FFlags Flags) {
   uint64_t RawFlags = 0;
   RawFlags |= Flags.ReadNone;
   RawFlags |= (Flags.ReadOnly << 1);
   RawFlags |= (Flags.NoRecurse << 2);
   RawFlags |= (Flags.ReturnDoesNotAlias << 3);
   RawFlags |= (Flags.NoInline << 4);
   RawFlags |= (Flags.AlwaysInline << 5);
+  RawFlags |= (Flags.NoUnwind << 6);
+  RawFlags |= (Flags.hasUnknownCall << 7);
   return RawFlags;
 }
 
 // Decode the flags for GlobalValue in the summary. See getDecodedGVSummaryFlags
 // in BitcodeReader.cpp.
 static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
   uint64_t RawFlags = 0;
 

llvm/lib/IR/AsmWriter.cpp

static const char *getVisibilityName(GlobalValue::VisibilityTypes Vis) {
   case GlobalValue::ProtectedVisibility:
     return "protected";
   }
   llvm_unreachable("invalid visibility");
 }
 
 void AssemblyWriter::printFunctionSummary(const FunctionSummary *FS) {
   Out << ", insts: " << FS->instCount();
+  if (FS->fflags().anyFlagSet())
+    Out << ", " << FS->fflags();
 
-  FunctionSummary::FFlags FFlags = FS->fflags();
-  if (FFlags.ReadNone | FFlags.ReadOnly | FFlags.NoRecurse |
-      FFlags.ReturnDoesNotAlias | FFlags.NoInline | FFlags.AlwaysInline) {
-    Out << ", funcFlags: (";
-    Out << "readNone: " << FFlags.ReadNone;
-    Out << ", readOnly: " << FFlags.ReadOnly;
-    Out << ", noRecurse: " << FFlags.NoRecurse;
-    Out << ", returnDoesNotAlias: " << FFlags.ReturnDoesNotAlias;
-    Out << ", noInline: " << FFlags.NoInline;
-    Out << ", alwaysInline: " << FFlags.AlwaysInline;
-    Out << ")";
-  }
   if (!FS->calls().empty()) {
     Out << ", calls: (";
     FieldSeparator IFS;
     for (auto &Call : FS->calls()) {
       Out << IFS;
       Out << "(callee: ^" << Machine.getGUIDSlot(Call.first.getGUID());
       if (Call.second.getHotness() != CalleeInfo::HotnessType::Unknown)
         Out << ", hotness: " << getHotnessName(Call.second.getHotness());

llvm/lib/IR/ModuleSummaryIndex.cpp

case GlobalValue::CommonLinkage:
     return "common";
   }
 
   return "<unknown>";
 }
 
 static std::string fflagsToString(FunctionSummary::FFlags F) {
   auto FlagValue = [](unsigned V) { return V ? '1' : '0'; };
-  char FlagRep[] = {FlagValue(F.ReadNone),     FlagValue(F.ReadOnly),
-                    FlagValue(F.NoRecurse),    FlagValue(F.ReturnDoesNotAlias),
-                    FlagValue(F.NoInline), FlagValue(F.AlwaysInline), 0};
+  char FlagRep[] = {FlagValue(F.ReadNone),  FlagValue(F.ReadOnly),
+                    FlagValue(F.NoRecurse), FlagValue(F.ReturnDoesNotAlias),
+                    FlagValue(F.NoInline),  FlagValue(F.AlwaysInline),
+                    FlagValue(F.NoUnwind),  0};
 
   return FlagRep;
 }
 
 // Get string representation of function instruction count and flags.
 static std::string getSummaryAttributes(GlobalValueSummary* GVS) {
   auto *FS = dyn_cast_or_null<FunctionSummary>(GVS);
   if (!FS)

llvm/lib/LTO/LTO.cpp

Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
   auto recordNewLinkage = [&](StringRef ModuleIdentifier,
                               GlobalValue::GUID GUID,
                               GlobalValue::LinkageTypes NewLinkage) {
     ResolvedODR[ModuleIdentifier][GUID] = NewLinkage;
   };
   thinLTOResolvePrevailingInIndex(Conf, ThinLTO.CombinedIndex, isPrevailing,
                                   recordNewLinkage, GUIDPreservedSymbols);
 
+  thinLTOPropagateFunctionAttrs(ThinLTO.CombinedIndex, isPrevailing);
+
   generateParamAccessSummary(ThinLTO.CombinedIndex);
 
   if (llvm::timeTraceProfilerEnabled())
     llvm::timeTraceProfilerEnd();
 
   TimeTraceScopeExit.release();
 
   std::unique_ptr<ThinBackendProc> BackendProc =

llvm/lib/LTO/LTOBackend.cpp

bool ClearDSOLocalOnDeclarations =
       TM->getRelocationModel() != Reloc::Static &&
       Mod.getPIELevel() == PIELevel::Default;
   renameModuleForThinLTO(Mod, CombinedIndex, ClearDSOLocalOnDeclarations);
 
   dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex);
 
   thinLTOResolvePrevailingInModule(Mod, DefinedGlobals);
 
+  thinLTOInsertFunctionAttrsForModule(Mod, DefinedGlobals);
+
   if (Conf.PostPromoteModuleHook && !Conf.PostPromoteModuleHook(Task, Mod))
     return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
 
   if (!DefinedGlobals.empty())
     thinLTOInternalizeModule(Mod, DefinedGlobals);
 
   if (Conf.PostInternalizeModuleHook &&
       !Conf.PostInternalizeModuleHook(Task, Mod))

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
+#include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/IPO/FunctionImport.h"
 #include "llvm/Transforms/IPO/Internalize.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/IPO/WholeProgramDevirt.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 
 #include <numeric>
bool ClearDSOLocalOnDeclarations =
       TheModule.getPIELevel() == PIELevel::Default;
 
   if (!SingleModule) {
     promoteModule(TheModule, Index, ClearDSOLocalOnDeclarations);
 
     // Apply summary-based prevailing-symbol resolution decisions.
     thinLTOResolvePrevailingInModule(TheModule, DefinedGlobals);
 
+    thinLTOInsertFunctionAttrsForModule(TheModule, DefinedGlobals);
+
     // Save temps: after promotion.
     saveTempBitcode(TheModule, SaveTempsDir, count, ".1.promoted.bc");
   }
 
   // Be friendly and don't nuke totally the module when the client didn't
   // supply anything to preserve.
   if (!ExportList.empty() || !GUIDPreservedSymbols.empty()) {
     // Apply summary-based internalization decisions.
void ThinLTOCodeGenerator::run() {
   // Changes are made in the index, consumed in the ThinLTO backends.
   updateIndexWPDForExports(*Index,
                            IsExported(ExportLists, GUIDPreservedSymbols),
                            LocalWPDTargetsMap);
   thinLTOInternalizeAndPromoteInIndex(
       *Index, IsExported(ExportLists, GUIDPreservedSymbols),
       IsPrevailing(PrevailingCopy));
 
+  thinLTOPropagateFunctionAttrs(*Index, IsPrevailing(PrevailingCopy));
+
   // Make sure that every module has an entry in the ExportLists, ImportList,
   // GVSummary and ResolvedODR maps to enable threaded access to these maps
   // below.
   for (auto &Module : Modules) {
     auto ModuleIdentifier = Module->getName();
     ExportLists[ModuleIdentifier];
     ImportLists[ModuleIdentifier];
     ResolvedODR[ModuleIdentifier];

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

 //===- FunctionAttrs.cpp - Pass which marks functions attributes ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file
 /// This file implements interprocedural passes which walk the
 /// call-graph deducing and/or propagating function attributes.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SCCIterator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/BasicAliasAnalysis.h"

 STATISTIC(NumNoAlias, "Number of function returns marked noalias");
 STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
 STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
 STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 STATISTIC(NumNoFree, "Number of functions marked as nofree");
 STATISTIC(NumWillReturn, "Number of functions marked as willreturn");
 STATISTIC(NumNoSync, "Number of functions marked as nosync");
 
+STATISTIC(NumThinLinkNoRecurse,
+          "Number of functions marked as norecurse during thinlink");
+STATISTIC(NumThinLinkNoUnwind,
+          "Number of functions marked as nounwind during thinlink");
+
 static cl::opt<bool> EnableNonnullArgPropagation(
     "enable-nonnull-arg-prop", cl::init(true), cl::Hidden,
     cl::desc("Try to propagate nonnull argument attributes from callsites to "
              "caller functions."));
 
 static cl::opt<bool> DisableNoUnwindInference(
     "disable-nounwind-inference", cl::Hidden,
     cl::desc("Stop inferring nounwind attribute during function-attrs pass"));
 
 static cl::opt<bool> DisableNoFreeInference(
     "disable-nofree-inference", cl::Hidden,
     cl::desc("Stop inferring nofree attribute during function-attrs pass"));
 
+static cl::opt<bool> DisableThinLTOPropagation(
+    "disable-thinlto-funcattrs", cl::init(true), cl::Hidden,
+    cl::desc("Don't propagate function-attrs in thinLTO"));
+
+static cl::opt<bool> OptimisticIndirectCalls(
+    "thinlto-funcattrs-optimistic-indirect", cl::init(false), cl::Hidden,
+    cl::desc("Optimistically assume indirect calls don't exist in thinlink "
+             "attribute propagation. Testing only"));
+
 namespace {
 
 using SCCNodeSet = SmallSetVector<Function *, 8>;
 
 } // end anonymous namespace
 
 /// Returns the memory access attribute for function F using AAR for AA results,
 /// where SCCNodes is the current SCC.
else if (ReadsMemory)
       ++NumReadOnly;
     else
       ++NumReadNone;
   }
 
   return MadeChange;
 }
 
+// Compute definitive function attributes for a function taking into account
+// prevailing definitions and linkage types
+static FunctionSummary *calculateDefinitiveAttributes(
+    ValueInfo VI, DenseMap<ValueInfo, FunctionSummary *> &CachedAttributes,
+    function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+        IsPrevailing) {
+
+  if (CachedAttributes.count(VI))
+    return CachedAttributes[VI];
+
+  /// At this point, prevailing symbols have been resolved. The following leads
+  /// to returning a conservative result:
+  /// - Multiple instances with local linkage. Normally local linkage would be
+  ///   unique per module
+  ///   as the GUID includes the module path. We could have a guid alias if
+  ///   there wasn't any distinguishing path when each file was compiled, but
+  ///   that should be rare so we'll punt on those.
+
+  /// These next 2 cases should not happen and will assert:
+  /// - Multiple instances with external linkage. This should be caught in
+  ///   symbol resolution
+  /// - Non-existent FunctionSummary for Aliasee. This presents a hole in our
+  ///   knowledge meaning we have to go conservative.
+
+  /// Otherwise, we calculate attributes for a function as:
+  ///   1. If we have a local linkage, take its attributes. If there's somehow
+  ///      multiple, bail and go conservative.
+  ///   2. If we have an external/WeakODR/LinkOnceODR linkage check that it is
+  ///      prevailing, take its attributes.
+  ///   3. If we have a Weak/LinkOnce linkage the copies can have semantic
+  ///      differences. However, if the prevailing copy is known it will be used
+  ///      so take its attributes. If the prevailing copy is in a native file
+  ///      all IR copies will be dead and propagation will go conservative.
+  ///   4. AvailableExternally summaries without a prevailing copy are known to
+  ///      occur in a couple of circumstances:
+  ///      a. An internal function gets imported due to its caller getting
+  ///         imported, it becomes AvailableExternally but no prevailing
+  ///         definition exists. Because it has to get imported along with its
+  ///         caller the attributes will be captured by propagating on its
+  ///         caller.
+  ///      b. C++11 [temp.explicit]p10 can generate AvailableExternally
+  ///         definitions of explicitly instanced template declarations
+  ///         for inlining which are ultimately dropped from the TU. Since this
+  ///         is localized to the TU the attributes will have already made it to
+  ///         the callers.
+  ///      These are edge cases and already captured by their callers so we
+  ///      ignore these for now. If they become relevant to optimize in the
+  ///      future this can be revisited.
+  ///   5. Otherwise, go conservative.
+
+  CachedAttributes[VI] = nullptr;
+  FunctionSummary *Local = nullptr;
+  FunctionSummary *Prevailing = nullptr;
+
+  for (const auto &GVS : VI.getSummaryList()) {
+    if (!GVS->isLive())
+      continue;
+
+    FunctionSummary *FS = dyn_cast<FunctionSummary>(GVS->getBaseObject());
+    // Virtual and Unknown (e.g. indirect) calls require going conservative
+    if (!FS || (!OptimisticIndirectCalls && FS->fflags().hasUnknownCall))
+      return nullptr;
+
+    const auto &Linkage = GVS->linkage();
+    if (GlobalValue::isLocalLinkage(Linkage)) {
+      if (Local) {
+        LLVM_DEBUG(
+            dbgs()
+            << "ThinLTO FunctionAttrs: Multiple Local Linkage, bailing on "
+               "function "
+            << VI.name() << " from " << FS->modulePath() << ". Previous module "
+            << Local->modulePath() << "\n");
+        return nullptr;
+      }
+      Local = FS;
+    } else if (GlobalValue::isExternalLinkage(Linkage)) {
+      assert(IsPrevailing(VI.getGUID(), GVS.get()));
+      Prevailing = FS;
+      break;
+    } else if (GlobalValue::isWeakODRLinkage(Linkage) ||
+               GlobalValue::isLinkOnceODRLinkage(Linkage) ||
+               GlobalValue::isWeakAnyLinkage(Linkage) ||
+               GlobalValue::isLinkOnceAnyLinkage(Linkage)) {
+      if (IsPrevailing(VI.getGUID(), GVS.get())) {
+        Prevailing = FS;
+        break;
+      }
+    } else if (GlobalValue::isAvailableExternallyLinkage(Linkage)) {
+      // TODO: Handle these cases if they become meaningful
+      continue;
+    }
+  }
+
+  if (Local) {
+    assert(!Prevailing);
+    CachedAttributes[VI] = Local;
+  } else if (Prevailing) {
+    assert(!Local);
+    CachedAttributes[VI] = Prevailing;
+  }
+
+  return CachedAttributes[VI];
+}
+
+bool llvm::thinLTOPropagateFunctionAttrs(
+    ModuleSummaryIndex &Index,
+    function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+        IsPrevailing) {
+  // TODO: implement addNoAliasAttrs once
+  // there's more information about the return type in the summary
+  if (DisableThinLTOPropagation)
+    return false;
+
+  DenseMap<ValueInfo, FunctionSummary *> CachedAttributes;
+  bool Changed = false;
+
+  auto PropagateAttributes = [&](std::vector<ValueInfo> &SCCNodes) {
+    // Assume we can propagate unless we discover otherwise
+    FunctionSummary::FFlags InferredFlags;
+    InferredFlags.NoRecurse = (SCCNodes.size() == 1);
+    InferredFlags.NoUnwind = true;
+
+    for (auto &V : SCCNodes) {
+      FunctionSummary *CallerSummary =
+          calculateDefinitiveAttributes(V, CachedAttributes, IsPrevailing);
+
+      // Function summaries can fail to contain information such as declarations
+      if (!CallerSummary)
+        return;
+
+      for (const auto &Callee : CallerSummary->calls()) {
+        FunctionSummary *CalleeSummary = calculateDefinitiveAttributes(
+            Callee.first, CachedAttributes, IsPrevailing);
+
+        if (!CalleeSummary)
+          return;
+
+        if (!CalleeSummary->fflags().NoRecurse)
+          InferredFlags.NoRecurse = false;
+
+        if (!CalleeSummary->fflags().NoUnwind)
+          InferredFlags.NoUnwind = false;
+
+        if (!InferredFlags.NoUnwind && !InferredFlags.NoRecurse)
+          break;
+      }
+    }
+
+    if (InferredFlags.NoUnwind || InferredFlags.NoRecurse) {
+      Changed = true;
+      for (auto &V : SCCNodes) {
+        if (InferredFlags.NoRecurse) {
+          LLVM_DEBUG(dbgs() << "ThinLTO FunctionAttrs: Propagated NoRecurse to "
+                            << V.name() << "\n");
+          ++NumThinLinkNoRecurse;
+          CachedAttributes[V]->setNoRecurse();
+        }
+
+        if (InferredFlags.NoUnwind) {
+          LLVM_DEBUG(dbgs() << "ThinLTO FunctionAttrs: Propagated NoUnwind to "
+                            << V.name() << "\n");
+          ++NumThinLinkNoUnwind;
+          CachedAttributes[V]->setNoUnwind();
+        }
+
+        for (auto &S : V.getSummaryList()) {
+          if (auto *FS = dyn_cast<FunctionSummary>(S.get())) {
+            if (InferredFlags.NoRecurse)
+              FS->setNoRecurse();
+
+            if (InferredFlags.NoUnwind)
+              FS->setNoUnwind();
+          }
+        }
+      }
+    }
+  };
+
+  // Call propagation functions on each SCC in the Index
+  for (scc_iterator<ModuleSummaryIndex *> I = scc_begin(&Index); !I.isAtEnd();
+       ++I) {
+    std::vector<ValueInfo> Nodes(*I);
+    PropagateAttributes(Nodes);
+  }
+  return Changed;
+}
+
+/// Insert function attributes in the Index back into the \p TheModule
+void llvm::thinLTOInsertFunctionAttrsForModule(
+    Module &TheModule, const GVSummaryMapTy &DefinedGlobals) {
+  if (DisableThinLTOPropagation)
+    return;
+
+  for (Function &F : TheModule) {
+    const auto &GV = DefinedGlobals.find(F.getGUID());
+    if (GV == DefinedGlobals.end())
+      continue;
+
+    if (FunctionSummary *FS = dyn_cast<FunctionSummary>(GV->second)) {
+      // TODO: propagate ReadNone and ReadOnly in thinlink.
+      if (FS->fflags().ReadNone && !F.doesNotAccessMemory())
+        F.setDoesNotAccessMemory();
+
+      if (FS->fflags().ReadOnly && !F.onlyReadsMemory())
+        F.setOnlyReadsMemory();
+
+      if (FS->fflags().NoRecurse && !F.doesNotRecurse())
+        F.setDoesNotRecurse();
+
+      if (FS->fflags().NoUnwind && !F.doesNotThrow())
+        F.setDoesNotThrow();
+    }
+  }
+}
+
 namespace {
 
 /// For a given pointer Argument, this retains a list of Arguments of functions
 /// in the same SCC that the pointer data flows into. We use this to build an
 /// SCC of the arguments.
 struct ArgumentGraphNode {
   Argument *Definition;
   SmallVector<ArgumentGraphNode *, 4> Uses;
 };
 
 class ArgumentGraph {
   // We store pointers to ArgumentGraphNode objects, so it's important that
   // that they not move around upon insert.
   using ArgumentMapTy = std::map<Argument *, ArgumentGraphNode>;
 
   ArgumentMapTy ArgumentMap;
 
   // There is no root node for the argument graph, in fact:
   //   void f(int *x, int *y) { if (...) f(x, y); }
   // is an example where the graph is disconnected. The SCCIterator requires a
   // single entry point, so we maintain a fake ("synthetic") root node that
   // uses every node. Because the graph is directed and nothing points into
   // the root, it will not participate in any SCCs (except for its own).
   ArgumentGraphNode SyntheticRoot;
 
 public:
   ArgumentGraph() { SyntheticRoot.Definition = nullptr; }
 
   using iterator = SmallVectorImpl<ArgumentGraphNode *>::iterator;
 
   iterator begin() { return SyntheticRoot.Uses.begin(); }
   iterator end() { return SyntheticRoot.Uses.end(); }
   ArgumentGraphNode *getEntryNode() { return &SyntheticRoot; }
 
   ArgumentGraphNode *operator[](Argument *A) {

llvm/test/Assembler/thinlto-summary.ll

 ^13 = gv: (guid: 12, summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0), varFlags: (readonly: 1))))
 
 ; Test a dsoLocal variable.
 ^14 = gv: (guid: 13, summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1), varFlags: (writeonly: 0))))
 
 ; Functions with various flag combinations (notEligibleToImport, Live,
 ; combinations of optional function flags).
 ^15 = gv: (guid: 14, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 1, dsoLocal: 0), insts: 1, funcFlags: (noInline: 1))))
-^16 = gv: (guid: 15, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1, funcFlags: (readNone: 1, noRecurse: 1, alwaysInline: 1))))
+^16 = gv: (guid: 15, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1, funcFlags: (readNone: 1, noRecurse: 1, alwaysInline: 1, noUnwind : 1, hasUnknownCall : 1))))
 ; This one also tests backwards reference in calls.
 ^17 = gv: (guid: 16, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 1, funcFlags: (readOnly: 1, returnDoesNotAlias: 1), calls: ((callee: ^15)))))
 
 ; Alias summary with backwards reference to aliasee.
 ^18 = gv: (guid: 17, summaries: (alias: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1), aliasee: ^14)))
 
 ; Test all types of TypeIdInfo on function summaries.
 ^19 = gv: (guid: 18, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0), insts: 4, typeIdInfo: (typeTests: (^24, ^26)))))

 ; CHECK: ^7 = gv: (guid: 6, summaries: (function: (module: ^0, flags: (linkage: linkonce, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^8 = gv: (guid: 7, summaries: (function: (module: ^0, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^9 = gv: (guid: 8, summaries: (function: (module: ^0, flags: (linkage: weak_odr, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 1), insts: 1)))
 ; CHECK: ^10 = gv: (guid: 9, summaries: (function: (module: ^0, flags: (linkage: weak, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1)))
 ; CHECK: ^11 = gv: (guid: 10, summaries: (variable: (module: ^0, flags: (linkage: common, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, constant: 0))))
 ; CHECK: ^12 = gv: (guid: 11, summaries: (variable: (module: ^0, flags: (linkage: appending, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, constant: 0), refs: (^4))))
 ; CHECK: ^13 = gv: (guid: 12, summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 1, writeonly: 0, constant: 0))))
 ; CHECK: ^14 = gv: (guid: 13, summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), varFlags: (readonly: 0, writeonly: 0, constant: 0))))
-; CHECK: ^15 = gv: (guid: 14, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 1, alwaysInline: 0))))
-; CHECK: ^16 = gv: (guid: 15, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 1, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 1))))
-; CHECK: ^17 = gv: (guid: 16, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 1, noRecurse: 0, returnDoesNotAlias: 1, noInline: 0, alwaysInline: 0), calls: ((callee: ^15)))))
+; CHECK: ^15 = gv: (guid: 14, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 1, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 1, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 0))))
+; CHECK: ^16 = gv: (guid: 15, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 1, readOnly: 0, noRecurse: 1, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 1, noUnwind: 1, hasUnknownCall: 1))))
+; CHECK: ^17 = gv: (guid: 16, summaries: (function: (module: ^1, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 1, noRecurse: 0, returnDoesNotAlias: 1, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 0), calls: ((callee: ^15)))))
 ; CHECK: ^18 = gv: (guid: 17, summaries: (alias: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), aliasee: ^14)))
 ; CHECK: ^19 = gv: (guid: 18, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, typeIdInfo: (typeTests: (^24, ^26)))))
 ; CHECK: ^20 = gv: (guid: 19, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 8, typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (^27, offset: 16))))))
 ; CHECK: ^21 = gv: (guid: 20, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadVCalls: (vFuncId: (^25, offset: 16))))))
 ; CHECK: ^22 = gv: (guid: 21, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, typeIdInfo: (typeTestAssumeConstVCalls: ((vFuncId: (^27, offset: 16), args: (42)), (vFuncId: (^27, offset: 24)))))))
 ; CHECK: ^23 = gv: (guid: 22, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadConstVCalls: ((vFuncId: (^28, offset: 16), args: (42)))))))
 ; CHECK: ^24 = typeid: (name: "_ZTS1C", summary: (typeTestRes: (kind: single, sizeM1BitWidth: 0))) ; guid = 1884921850105019584
 ; CHECK: ^25 = typeid: (name: "_ZTS1B", summary: (typeTestRes: (kind: inline, sizeM1BitWidth: 0, alignLog2: 1, sizeM1: 2, bitMask: 3, inlineBits: 4))) ; guid = 6203814149063363976

llvm/test/Bitcode/thinlto-function-summary-refgraph.ll

 ; DIS-DAG: = gv: (name: "W", summaries: (function: (module: ^0, flags: (linkage: weak_odr, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 2, calls: ((callee: ^{{.*}})), refs: (^{{.*}})))) ; guid = 5790125716599269729
 ; DIS-DAG: = gv: (name: "foo") ; guid = 6699318081062747564
 ; DIS-DAG: = gv: (name: "func") ; guid = 7289175272376759421
 ; DIS-DAG: = gv: (name: "func3") ; guid = 11517462787082255043
 ; Check that default value of writeonly attribute is zero for constant variables 
 ; DIS-DAG: = gv: (name: "globalvar", summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 1, writeonly: 0, constant: 1)))) ; guid = 12887606300320728018
 ; DIS-DAG: = gv: (name: "func2") ; guid = 14069196320850861797
 ; DIS-DAG: = gv: (name: "llvm.ctpop.i8") ; guid = 15254915475081819833
-; DIS-DAG: = gv: (name: "main", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 9, calls: ((callee: ^{{.*}})), refs: (^{{.*}})))) ; guid = 15822663052811949562
+; DIS-DAG: = gv: (name: "main", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 9, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), calls: ((callee: ^{{.*}})), refs: (^{{.*}})))) ; guid = 15822663052811949562
 ; DIS-DAG: = gv: (name: "bar", summaries: (variable: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), varFlags: (readonly: 1, writeonly: 1, constant: 0),  refs: (^{{.*}})))) ; guid = 16434608426314478903
 ; Don't try to match the exact GUID. Since it is private, the file path
 ; will get hashed, and that will be test dependent.
 ; DIS-DAG: = gv: (name: "Y", summaries: (function: (module: ^0, flags: (linkage: private, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 1, canAutoHide: 0), insts: 14, calls: ((callee: ^{{.*}}))))) ; guid =

llvm/test/Bitcode/thinlto-type-vcalls.ll

 }
 
 declare i1 @llvm.type.test(i8*, metadata) nounwind readnone
 declare void @llvm.assume(i1)
 declare {i8*, i1} @llvm.type.checked.load(i8*, i32, metadata)
 
 ; DIS: ^0 = module: (path: "{{.*}}", hash: (0, 0, 0, 0, 0))
 ; DIS: ^1 = gv: (name: "llvm.type.test") ; guid = 608142985856744218
-; DIS: ^2 = gv: (name: "f1", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 8, typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16)))))) ; guid = 2072045998141807037
-; DIS: ^3 = gv: (name: "f3", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16)))))) ; guid = 4197650231481825559
+; DIS: ^2 = gv: (name: "f1", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 8, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16)))))) ; guid = 2072045998141807037
+; DIS: ^3 = gv: (name: "f3", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeCheckedLoadVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16)))))) ; guid = 4197650231481825559
 ; DIS: ^4 = gv: (name: "llvm.type.checked.load") ; guid = 5568222536364573403
 ; DIS: ^5 = gv: (name: "llvm.assume") ; guid = 6385187066495850096
-; DIS: ^6 = gv: (name: "f2", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 24), vFuncId: (guid: 16434608426314478903, offset: 32)))))) ; guid = 8471399308421654326
-; DIS: ^7 = gv: (name: "f4", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, typeIdInfo: (typeTestAssumeConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)), (vFuncId: (guid: 6699318081062747564, offset: 24), args: (43))))))) ; guid = 10064745020953272174
-; DIS: ^8 = gv: (name: "f5", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42))))))) ; guid = 11686717102184386164
+; DIS: ^6 = gv: (name: "f2", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 24), vFuncId: (guid: 16434608426314478903, offset: 32)))))) ; guid = 8471399308421654326
+; DIS: ^7 = gv: (name: "f4", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)), (vFuncId: (guid: 6699318081062747564, offset: 24), args: (43))))))) ; guid = 10064745020953272174
+; DIS: ^8 = gv: (name: "f5", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeCheckedLoadConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42))))))) ; guid = 11686717102184386164
 ; DIS: ^9 = gv: (name: "f6", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 2, typeIdInfo: (typeTests: (7546896869197086323))))) ; guid = 11834966808443348068
 
 ; COMBINED-DIS: ^0 = module: (path: "{{.*}}thinlto-type-vcalls.ll.tmp.o", hash: (0, 0, 0, 0, 0))
-; COMBINED-DIS: ^1 = gv: (guid: 2072045998141807037, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 8, typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16))))))
-; COMBINED-DIS: ^2 = gv: (guid: 4197650231481825559, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16))))))
-; COMBINED-DIS: ^3 = gv: (guid: 8471399308421654326, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 24), vFuncId: (guid: 16434608426314478903, offset: 32))))))
-; COMBINED-DIS: ^4 = gv: (guid: 10064745020953272174, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, typeIdInfo: (typeTestAssumeConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)), (vFuncId: (guid: 6699318081062747564, offset: 24), args: (43)))))))
-; COMBINED-DIS: ^5 = gv: (guid: 11686717102184386164, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, typeIdInfo: (typeCheckedLoadConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)))))))
+; COMBINED-DIS: ^1 = gv: (guid: 2072045998141807037, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 8, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16))))))
+; COMBINED-DIS: ^2 = gv: (guid: 4197650231481825559, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeCheckedLoadVCalls: (vFuncId: (guid: 6699318081062747564, offset: 16))))))
+; COMBINED-DIS: ^3 = gv: (guid: 8471399308421654326, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeVCalls: (vFuncId: (guid: 6699318081062747564, offset: 24), vFuncId: (guid: 16434608426314478903, offset: 32))))))
+; COMBINED-DIS: ^4 = gv: (guid: 10064745020953272174, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 15, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeTestAssumeConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)), (vFuncId: (guid: 6699318081062747564, offset: 24), args: (43)))))))
+; COMBINED-DIS: ^5 = gv: (guid: 11686717102184386164, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 5, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, hasUnknownCall: 1), typeIdInfo: (typeCheckedLoadConstVCalls: ((vFuncId: (guid: 6699318081062747564, offset: 16), args: (42)))))))
 ; COMBINED-DIS: ^6 = gv: (guid: 11834966808443348068, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 2, typeIdInfo: (typeTests: (7546896869197086323)))))

llvm/test/ThinLTO/X86/deadstrip.ll

 ; RUN:   -r %t1.bc,_linkonceodrfuncwithalias,l \
 ; RUN:   -r %t1.bc,_linkonceodrfuncwithalias_caller,pl \
 ; RUN:   -r %t2.bc,_baz,pl \
 ; RUN:   -r %t2.bc,_boo,pl \
 ; RUN:   -r %t2.bc,_dead_func,l \
 ; RUN:   -r %t2.bc,_another_dead_func,pl \
 ; RUN:   -r %t2.bc,_linkonceodrfuncwithalias,pl \
 ; RUN:   -thinlto-threads=1 \
+; RUN:   -disable-thinlto-funcattrs=0 \
 ; RUN:   -debug-only=function-import 2>&1 | FileCheck %s --check-prefix=DEBUG --check-prefix=STATS
 ; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=LTO2
 ; RUN: llvm-dis < %t.out.2.3.import.bc | FileCheck %s --check-prefix=LTO2-CHECK2
 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM
 
 ; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=COMBINED
 ; Live, NotEligibleForImport, dso_local, Internal
 ; COMBINED-DAG: <COMBINED {{.*}} op2=119

 ; CHECK: define internal void @_GLOBAL__I_a()
 ; CHECK: define internal void @bar() {
 ; CHECK: define internal void @bar_internal()
 ; CHECK: define internal void @dead_func() {
 ; CHECK-NOT: available_externally {{.*}} @baz()
 ; LTO2-NOT: available_externally {{.*}} @baz()
 ; LTO2: @llvm.global_ctors =
 ; LTO2: define internal void @_GLOBAL__I_a()
-; LTO2: define internal void @bar() {
+; LTO2: define internal void @bar() [[ATTR:#[0-9]+]] {
 ; LTO2: define internal void @bar_internal()
 ; LTO2-NOT: @dead_func()
 ; LTO2-NOT: available_externally {{.*}} @baz()
 
 ; Make sure we didn't internalize @boo, which is reachable via
 ; llvm.global_ctors
 ; CHECK2: define void @boo()
 ; LTO2-CHECK2: define dso_local void @boo()
 
 ; Make sure we keep @linkonceodrfuncwithalias in Input/deadstrip.ll alive as it
 ; is reachable from @main.
-; LTO2-CHECK2: define weak_odr dso_local void @linkonceodrfuncwithalias() {
+; LTO2-CHECK2: define weak_odr dso_local void @linkonceodrfuncwithalias() [[ATTR:#[0-9]+]] {
 
 ; We should have eventually removed @baz since it was internalized and unused
 ; CHECK2-NM-NOT: _baz
 
 ; The final binary should not contain any of the dead functions,
 ; only main is expected because bar is expected to be inlined and stripped out.
 ; CHECK-NM-NOT: bar
 ; CHECK-NM-NOT: dead
 ; CHECK-NM: T _main
 ; CHECK-NM-NOT: bar
 ; CHECK-NM-NOT: dead
 
 ; DEBUG-DAG: Live root: 2412314959268824392 (llvm.global_ctors)
 ; DEBUG-DAG: Live root: 15822663052811949562 (main)
 ; DEBUG-DAG: Ignores Dead GUID: 7342339837106705152 (dead_func)
 ; DEBUG-DAG: Ignores Dead GUID: 7546896869197086323 (baz)
 ; DEBUG-DAG: Initialize import for 15611644523426561710 (boo)
 ; DEBUG-DAG: Ignores Dead GUID: 2384416018110111308 (another_dead_func)
 
+; LTO2-DAG: attributes [[ATTR]] = { norecurse nounwind }
+
 ; STATS: 3 function-import  - Number of dead stripped symbols in index
 
 ; Next test the case where Inputs/deadstrip.ll does not get a module index,
 ; which will cause it to be handled by regular LTO in the new LTO API.
 ; In that case there are uses of @dead_func in the regular LTO partition
 ; and it shouldn't be internalized.
 ; RUN: opt %p/Inputs/deadstrip.ll -o %t3.bc
 ; RUN: llvm-lto2 run %t1.bc %t3.bc -o %t4.out -save-temps \

llvm/test/ThinLTO/X86/dot-dumper.ll

 ; PERMODULE: digraph Summary {
 ; PERMODULE-NEXT:  // Module: 
 ; PERMODULE-NEXT:  subgraph cluster_0 {
 ; PERMODULE-NEXT:    style = filled;
 ; PERMODULE-NEXT:    color = lightgrey;
 ; PERMODULE-NEXT:    label = "";
 ; PERMODULE-NEXT:    node [style=filled,fillcolor=lightblue];
 ; PERMODULE-NEXT:    M0_[[MAIN_ALIAS:[0-9]+]] [style="dotted,filled",shape="box",label="main_alias",fillcolor="red"]; // alias, dead
-; PERMODULE-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 4, ffl: 000000)}",fillcolor="red"]; // function, dead
+; PERMODULE-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 4, ffl: 0000000)}",fillcolor="red"]; // function, dead
 ; PERMODULE-NEXT:    // Edges:
 ; PERMODULE-NEXT:    M0_[[MAIN_ALIAS]] -> M0_[[MAIN]] [style=dotted]; // alias
 ; PERMODULE-NEXT:  }
 ; PERMODULE-NEXT:  // Cross-module edges:
 ; PERMODULE-NEXT:  [[A:[0-9]+]] [label="A"]; // defined externally
 ; PERMODULE-NEXT:  M0_[[MAIN]] -> [[A]] [style=dashed,color=forestgreen]; // const-ref
 ; PERMODULE-NEXT:  [[FOO:[0-9]+]] [label="foo"]; // defined externally
 ; PERMODULE-NEXT:  M0_[[MAIN]] -> [[FOO]] // call (hotness : Unknown)
 ; PERMODULE-NEXT: }
 
 ; COMBINED:     digraph Summary {
 ; COMBINED-NEXT:  // Module: {{.*}}dot-dumper{{.*}}1.bc
 ; COMBINED-NEXT:  subgraph cluster_0 {
 ; COMBINED-NEXT:    style = filled;
 ; COMBINED-NEXT:    color = lightgrey;
 ; COMBINED-NEXT:    label = "dot-dumper{{.*}}1.bc";
 ; COMBINED-NEXT:    node [style=filled,fillcolor=lightblue];
 ; COMBINED-NEXT:    M0_[[MAIN_ALIAS:[0-9]+]] [style="dotted,filled",shape="box",label="main_alias",fillcolor="red"]; // alias, dead
-; COMBINED-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 4, ffl: 000000)}"]; // function, preserved
+; COMBINED-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 4, ffl: 0000000)}"]; // function, preserved
 ; COMBINED-NEXT:    // Edges:
 ; COMBINED-NEXT:    M0_[[MAIN_ALIAS]] -> M0_[[MAIN]] [style=dotted]; // alias
 ; COMBINED-NEXT:  }
 ; COMBINED-NEXT:  // Module: {{.*}}dot-dumper{{.*}}2.bc
 ; COMBINED-NEXT:  subgraph cluster_1 {
 ; COMBINED-NEXT:    style = filled;
 ; COMBINED-NEXT:    color = lightgrey;
 ; COMBINED-NEXT:    label = "dot-dumper{{.*}}2.bc";
 ; COMBINED-NEXT:    node [style=filled,fillcolor=lightblue];
-; COMBINED-NEXT:    M1_[[FOO:[0-9]+]] [shape="record",label="foo|extern (inst: 4, ffl: 000010)}"]; // function
+; COMBINED-NEXT:    M1_[[FOO:[0-9]+]] [shape="record",label="foo|extern (inst: 4, ffl: 0000100)}"]; // function
 ; COMBINED-NEXT:    M1_[[A:[0-9]+]] [shape="Mrecord",label="A|extern}"]; // variable, immutable
 ; COMBINED-NEXT:    M1_[[B:[0-9]+]] [shape="Mrecord",label="B|extern}"]; // variable, immutable, constant
-; COMBINED-NEXT:    M1_{{[0-9]+}} [shape="record",label="bar|extern (inst: 1, ffl: 000000)}",fillcolor="red"]; // function, dead
+; COMBINED-NEXT:    M1_{{[0-9]+}} [shape="record",label="bar|extern (inst: 1, ffl: 0000000)}",fillcolor="red"]; // function, dead
 ; COMBINED-NEXT:    // Edges:
 ; COMBINED-NEXT:    M1_[[FOO]] -> M1_[[B]] [style=dashed,color=forestgreen]; // const-ref
 ; COMBINED-NEXT:    M1_[[FOO]] -> M1_[[A]] [style=dashed,color=forestgreen]; // const-ref
 ; COMBINED-NEXT:  }
 ; COMBINED-NEXT:  // Cross-module edges:
 ; COMBINED-NEXT:  M0_[[MAIN]] -> M1_[[A]] [style=dashed,color=forestgreen]; // const-ref
 ; COMBINED-NEXT:  M0_[[MAIN]] -> M1_[[FOO]] // call (hotness : Unknown)
 ; COMBINED-NEXT: }

llvm/test/ThinLTO/X86/dot-dumper2.ll

 
 ; COMBINED: digraph Summary {
 ; COMBINED-NEXT:  // Module:
 ; COMBINED-NEXT:  subgraph cluster_0 {
 ; COMBINED-NEXT:    style = filled;
 ; COMBINED-NEXT:    color = lightgrey;
 ; COMBINED-NEXT:    label =
 ; COMBINED-NEXT:    node [style=filled,fillcolor=lightblue];
-; COMBINED-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 2, ffl: 000000)}"]; // function
+; COMBINED-NEXT:    M0_[[MAIN:[0-9]+]] [shape="record",label="main|extern (inst: 2, ffl: 0000000)}"]; // function
 ; COMBINED-NEXT:    // Edges:
 ; COMBINED-NEXT:  }
 ; COMBINED-NEXT:  // Module:
 ; COMBINED-NEXT:  subgraph cluster_1 {
 ; COMBINED-NEXT:    style = filled;
 ; COMBINED-NEXT:    color = lightgrey;
 ; COMBINED-NEXT:    label =
 ; COMBINED-NEXT:    node [style=filled,fillcolor=lightblue];

llvm/test/ThinLTO/X86/funcattrs-prop-exported-internal.ll

+; Function import can promote an internal function to external but not mark it as prevailing.
+; Given that the internal function's attributes would have already propagated to its callers 
+; that are part of the import chain there's no need to actually propagate off this copy as 
+; propagating the caller performs the same thing.
+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc %t/main.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
+; RUN: opt -thinlto-bc %t/callees.ll -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 \
+; RUN:   %t1.bc %t2.bc -o %t.o \
+; RUN:   -r %t1.bc,caller,l -r %t1.bc,caller_noattr,l -r %t1.bc,importer,px -r %t1.bc,importer_noattr,px \
+; RUN:   -r %t2.bc,caller,px -r %t2.bc,caller_noattr,px \
+; RUN:   -save-temps
+; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s --match-full-lines
+
+;--- main.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @caller()
+declare void @caller_noattr()
+
+; CHECK: define void @importer() [[ATTR_PROP:#[0-9]+]] {
+define void @importer() {
+  call void @caller()
+  ret void
+}
+
+; If somehow the caller doesn't get the attributes, we
+; shouldn't propagate from the internal callee.
+; CHECK: define void @importer_noattr() {
+define void @importer_noattr() {
+  call void @caller_noattr()
+  ret void
+}
+
+; CHECK: define available_externally hidden void @callee{{.*}}
+
+; CHECK-DAG: attributes [[ATTR_PROP]] = { norecurse nounwind }
+
+;--- callees.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+attributes #0 = { nounwind norecurse }
+
+define void @caller() #0 {
+  call void @callee()
+  ret void
+}
+
+define void @caller_noattr() {
+  call void @callee()
+  ret void
+}
+
+define internal void @callee() #0 {
+  ret void
+}

llvm/test/ThinLTO/X86/funcattrs-prop-indirect.ll

+; Indirect calls returns conservative results from function propagation
+; RUN: opt -thinlto-bc %s -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc -o %t.o -r %t1.bc,indirect,px -save-temps
+; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-NOT: ; Function Attrs: norecurse nounwind
+; CHECK: define i32 @indirect(i32 ()* nocapture %0) {
+define i32 @indirect(i32 ()* nocapture) {
+  %2 = tail call i32 %0()
+  ret i32 %2
+}

llvm/test/ThinLTO/X86/funcattrs-prop-undefined.ll

+; Callee1 isn't defined, propagation goes conservative
+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc %t/main.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
+; RUN: opt -thinlto-bc %t/callees.ll -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -o %t.o -r %t1.bc,caller,px -r %t1.bc,callee,l -r %t1.bc,callee1,l -r %t2.bc,callee,px -save-temps
+; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
+
+;--- main.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @callee()
+declare void @callee1()
+
+; CHECK-NOT: Function Attrs: 
+; CHECK: define void @caller()
+define void @caller() {
+  call void @callee()
+  call void @callee1()
+  ret void
+}
+
+;--- callees.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+attributes #0 = { nounwind norecurse }
+
+define void @callee() #0 {
+  ret void
+}

llvm/test/ThinLTO/X86/funcattrs-prop-weak.ll

+; RUN: split-file %s %t
+; RUN: opt -thinlto-bc %t/a.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
+; RUN: opt -thinlto-bc %t/b.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t2.bc
+; RUN: opt -thinlto-bc %t/c.ll -thin-link-bitcode-file=%t1.thinlink.bc -o %t3.bc
+
+; If the prevailing weak symbol is defined in a native file, the IR copies should be dead and propagation should not occur
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc %t3.bc -o %t.o \
+; RUN:               -r %t1.bc,caller,px -r %t1.bc,callee,lx \
+; RUN:               -r %t2.bc,callee,x \
+; RUN:               -r %t3.bc,callee,x \
+; RUN:               -save-temps
+
+; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
+
+; If the prevailing weak symbol is in an IR file, it should be the one used in the final binary and thus propagation
+; should be based off of that copy
+; RUN: llvm-lto2 run -O3 -disable-thinlto-funcattrs=0 %t1.bc %t2.bc %t3.bc -o %t.2.o \
+; RUN:               -r %t1.bc,caller,px -r %t1.bc,callee,lx \
+; RUN:               -r %t2.bc,callee,px \
+; RUN:               -r %t3.bc,callee,x \
+; RUN:               -save-temps
+
+; RUN: llvm-dis -o - %t.2.o.1.3.import.bc | FileCheck %s --check-prefix=PREVAILING
+; RUN: llvm-dis -o - %t.2.o.2.3.import.bc | FileCheck %s --check-prefix=PREVAILING-B
+; RUN: llvm-dis -o - %t.2.o.3.3.import.bc | FileCheck %s --check-prefix=PREVAILING-C
+
+;--- a.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @callee()
+
+; CHECK-NOT: Function Attrs: 
+; CHECK: define i32 @caller()
+
+; PREVAILING: Function Attrs: norecurse nounwind
+; PREVAILING-NEXT: define i32 @caller()
+define i32 @caller() {
+  %res = call i32 @callee()
+  ret i32 %res
+}
+
+;--- b.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; PREVAILING-B: define weak i32 @callee()
+define weak i32 @callee() {
+  ret i32 5
+}
+
+;--- c.ll
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; PREVAILING-C: declare i32 @callee()
+define weak i32 @callee() {
+  ret i32 6
+}
+

llvm/test/ThinLTO/X86/funcattrs-prop.ll

+; RUN: split-file %s %t
+; RUN: opt -module-summary %t/a.ll -o %t/a.bc
+; RUN: opt -module-summary %t/b.ll -o %t/b.bc
+; RUN: opt -module-summary %t/c.ll -o %t/c.bc
+
+;; Function attribute propagation
+;; 1. If external, linkonce_odr or weak_odr. We capture the attributes of the prevailing symbol for propagation.
+;; 2. If linkonce or weak, individual callers may optimize using different copies. However, the prevailing symbol will be what's in the final binary and thus we can propagate based off of that
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t/a.bc %t/b.bc %t/c.bc -o %t1 -save-temps \
+; RUN:   -r=%t/a.bc,call_extern,plx -r=%t/a.bc,call_linkonceodr,plx -r=%t/a.bc,call_weakodr,plx -r=%t/a.bc,call_linkonce,plx -r=%t/a.bc,call_weak,plx -r=%t/a.bc,call_linkonce_may_unwind,plx -r=%t/a.bc,call_weak_may_unwind,plx \
+; RUN:   -r=%t/a.bc,extern, -r=%t/a.bc,linkonceodr, -r=%t/a.bc,weakodr, -r=%t/a.bc,linkonce, -r=%t/a.bc,weak, -r=%t/a.bc,linkonce_may_unwind, -r=%t/a.bc,weak_may_unwind, \
+; RUN:   -r=%t/b.bc,extern,p -r=%t/b.bc,linkonceodr,p -r=%t/b.bc,weakodr,p -r=%t/b.bc,linkonce,p -r=%t/b.bc,weak,p -r=%t/b.bc,linkonce_may_unwind,p -r=%t/b.bc,weak_may_unwind, \
+; RUN:   -r=%t/c.bc,extern, -r=%t/c.bc,linkonceodr, -r=%t/c.bc,weakodr, -r=%t/c.bc,linkonce, -r=%t/c.bc,weak, -r=%t/c.bc,linkonce_may_unwind, -r=%t/c.bc,weak_may_unwind,p -r=%t/c.bc,may_throw,
+
+; RUN: llvm-dis %t1.1.3.import.bc -o - | FileCheck %s
+
+;--- a.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @extern()
+declare void @linkonceodr()
+declare void @weakodr()
+
+declare void @linkonce()
+declare void @weak()
+
+; b.ll contains non-recursing copies
+; c.ll contains recursing copies
+declare void @linkonce_may_unwind()
+declare void @weak_may_unwind()
+
+; CHECK: define dso_local void @call_extern() [[ATTR_PROP:#[0-9]+]]
+define void @call_extern() {
+    call void @extern()
+    ret void
+}
+
+; CHECK: define dso_local void @call_linkonceodr() [[ATTR_PROP:#[0-9]+]]
+define void @call_linkonceodr() {
+    call void @linkonceodr()
+    ret void
+}
+
+; CHECK: define dso_local void @call_weakodr() [[ATTR_PROP:#[0-9]+]]
+define void @call_weakodr() {
+    call void @weakodr()
+    ret void
+}
+
+; CHECK: define dso_local void @call_linkonce() [[ATTR_PROP:#[0-9]+]]
+define void @call_linkonce() {
+    call void @linkonce()
+    ret void
+}
+
+; CHECK: define dso_local void @call_weak() [[ATTR_PROP:#[0-9]+]]
+define void @call_weak() {
+    call void @weak()
+    ret void
+}
+
+; CHECK: define dso_local void @call_linkonce_may_unwind() [[ATTR_PROP:#[0-9]+]]
+define void @call_linkonce_may_unwind() {
+    call void @linkonce_may_unwind()
+    ret void
+}
+
+; CHECK: define dso_local void @call_weak_may_unwind() [[ATTR_NORECURSE:#[0-9]+]]
+define void @call_weak_may_unwind() {
+    call void @weak_may_unwind()
+    ret void
+}
+
+; CHECK-DAG: attributes [[ATTR_PROP]] = { norecurse nounwind }
+; CHECK-DAG: attributes [[ATTR_NORECURSE]] = { norecurse }
+
+;--- b.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+attributes #0 = { nounwind norecurse }
+
+define void @extern() #0 {
+  ret void
+}
+
+define linkonce_odr void @linkonceodr() #0 {
+  ret void
+}
+
+define weak_odr void @weakodr() #0 {
+  ret void
+}
+
+define linkonce void @linkonce() #0 {
+  ret void
+}
+
+define weak void @weak() #0 {
+  ret void
+}
+
+define linkonce void @linkonce_may_unwind() #0 {
+  ret void
+}
+
+define weak void @weak_may_unwind() #0 {
+  ret void
+}
+
+;--- c.ll
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+attributes #0 = { nounwind norecurse }
+attributes #1 = { norecurse }
+
+define void @extern() #0 {
+  ret void
+}
+
+define linkonce_odr void @linkonceodr() #0 {
+  ret void
+}
+
+define weak_odr void @weakodr() #0 {
+  ret void
+}
+
+define linkonce void @linkonce() #0 {
+  ret void
+}
+
+define weak void @weak() #0 {
+  ret void
+}
+
+declare void @may_throw()
+
+define linkonce void @linkonce_may_unwind() #1 {
+  call void @may_throw()
+  ret void
+}
+
+define weak void @weak_may_unwind() #1 {
+  call void @may_throw()
+  ret void
+}

llvm/test/ThinLTO/X86/funcimport_alwaysinline.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/funcimport_alwaysinline.ll -o %t2.bc
 
-; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN: llvm-lto2 run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -o %t.o -save-temps \
 ; RUN:     -r=%t1.bc,foo,plx \
 ; RUN:     -r=%t2.bc,main,plx \
 ; RUN:     -r=%t2.bc,foo,l \
 ; RUN:     -import-instr-limit=0
 ; RUN: llvm-dis %t.o.2.3.import.bc -o - | FileCheck %s --check-prefix=CHECK1
 ; RUN: llvm-dis %t.o.index.bc -o - | FileCheck %s --check-prefix=CHECK2
 
 ; foo() being always_inline should be imported irrespective of the
 ; instruction limit
 ; CHECK1: define available_externally void @foo()
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: alwaysinline nounwind uwtable
 define void @foo() #0 {
 entry:
   ret void
 }
 
 attributes #0 = { alwaysinline nounwind uwtable }
-; CHECK2: ^2 = gv: (guid: {{.*}}, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 1))))
+; CHECK2: ^2 = gv: (guid: {{.*}}, summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 1, noUnwind: 1, hasUnknownCall: 0))))

llvm/test/ThinLTO/X86/function_entry_count.ll

 ; RUN: opt -thinlto-bc %s -write-relbf-to-summary -thin-link-bitcode-file=%t1.thinlink.bc -o %t1.bc
 ; RUN: opt -thinlto-bc %p/Inputs/function_entry_count.ll -write-relbf-to-summary -thin-link-bitcode-file=%t2.thinlink.bc -o %t2.bc
 
 ; First perform the thin link on the normal bitcode file.
-; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -thinlto-synthesize-entry-counts \
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts \
 ; RUN:     -r=%t1.bc,g, \
 ; RUN:     -r=%t1.bc,f,px \
 ; RUN:     -r=%t1.bc,h,px \
 ; RUN:     -r=%t2.bc,h, \
 ; RUN:     -r=%t2.bc,g,px
 ; RUN: llvm-dis -o - %t.o.1.3.import.bc | FileCheck %s
 
-; RUN: llvm-lto -thinlto-action=run -thinlto-synthesize-entry-counts -exported-symbol=f \
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 -thinlto-synthesize-entry-counts -exported-symbol=f \
 ; RUN:     -exported-symbol=g -exported-symbol=h -thinlto-save-temps=%t3. %t1.bc %t2.bc
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s
 
-; CHECK: define void @h() !prof ![[PROF2:[0-9]+]]
-; CHECK: define void @f(i32{{.*}}) !prof ![[PROF1:[0-9]+]]
+; CHECK: define void @h() [[ATTR:#[0-9]+]] !prof ![[PROF2:[0-9]+]]
+; CHECK: define void @f(i32{{.*}}) [[ATTR:#[0-9]+]] !prof ![[PROF1:[0-9]+]]
 ; CHECK: define available_externally void @g() !prof ![[PROF2]]
 ; CHECK-DAG: ![[PROF1]] = !{!"synthetic_function_entry_count", i64 10}
 ; CHECK-DAG: ![[PROF2]] = !{!"synthetic_function_entry_count", i64 198}
+; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 declare void @g();
 
 define void @h() {
   ret void

llvm/test/ThinLTO/X86/linkonce_resolution_comdat.ll

 ; This test ensures that we drop the preempted copy of @f from %t2.bc from its
 ; comdat after making it available_externally. If not we would get a
 ; verification error.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/linkonce_resolution_comdat.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=run %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
+; RUN: llvm-lto -thinlto-action=run -disable-thinlto-funcattrs=0 %t1.bc %t2.bc -exported-symbol=f -exported-symbol=g -thinlto-save-temps=%t3.
 
 ; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT1
 ; RUN: llvm-dis %t3.1.3.imported.bc -o - | FileCheck %s --check-prefix=IMPORT2
 ; Copy from first module is prevailing and converted to weak_odr, copy
 ; from second module is preempted and converted to available_externally and
 ; removed from comdat.
-; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr comdat($c1) {
-; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr {
+; IMPORT1: define weak_odr i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] comdat($c1) {
+; IMPORT2: define available_externally i32 @f(i8* %0) unnamed_addr [[ATTR:#[0-9]+]] {
+
+; CHECK-DAG: attributes [[ATTR]] = { norecurse nounwind }
 
 ; RUN: llvm-nm -o - < %t1.bc.thinlto.o | FileCheck %s --check-prefix=NM1
 ; NM1: W f
 
 ; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=NM2
 ; f() would have been turned into available_externally since it is preempted,
 ; and inlined into g()
 ; NM2-NOT: f