[ThinLTO] Internalize read only globals

[evgeny777]

Summary

One of annoying problems in ThinLTO is that variable promotion can prevent optimiser from constant folding/propagation. Consider
following C program

main.c

int main() {
  int foo();
  return foo();
}

foo.c

#include <stdlib.h>

static int gFoo = 1;

int foo() {
  return gFoo;
}

void bar() {
  gFoo = rand();
}

Note that bar() is dead, so variable gFoo is never really modified. However promotion of gFoo to hidden global makes it impossible for optimizer to mark it constant and fold it afterwards.

To overcome this problem I suggest to introduce concept of "constant reference" which implies all accesses to a given global from a given function are non-volatile loads. With this we can determine which variables will become read-only after DCE and convert them to constants with a special LLVM pass.

This patch implements first part of this approach. It doesn't have good test cases yet - for now I'm just interested of your opinion.

[tejohnson]

Awesome, thanks! This was future work that we didn't have the bandwidth to address. A few comments/suggestions:

I think it would be cleaner/clearer to encode this info along with the Ref edges. Similar to how there is callee info for each call edge, we should have a ref info that for now can just be this bit.

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

Will we miss some cases if the reference is on something like a bitcast that feeds a non-volatile load?

[steven_wu]

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

What if the constant is really big? Looks like the importing decision has to be made by the thin linker directly. You can not import the "constant" global as something like "available_externally constant" because the original copy might not be visible from the original module (just like the example), unless you want to modify the original copy to be "external hidden".

[tejohnson]

In D49362#1163768, @steven_wu wrote:

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

What if the constant is really big? Looks like the importing decision has to be made by the thin linker directly. You can not import the "constant" global as something like "available_externally constant" because the original copy might not be visible from the original module (just like the example), unless you want to modify the original copy to be "external hidden".

Not sure I understand your question, can you clarify the concern? Right now, computeImportForReferencedGlobals during the thin link we import any referenced variable, unless it has interposable linkage or references other summaries (in which case it is not a constant). The imported variables are imported as available externally definitions. The original definition is currently promoted to external hidden.

[steven_wu]

In D49362#1163775, @tejohnson wrote:

In D49362#1163768, @steven_wu wrote:

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type. That will avoid the need to do any modification to the optimization passes. On the exporting side, we could do prevent the promotion if we knew that all modules importing a reference also imported the referenced variable definition. It does however look like we should be importing all non-interposable linkage constant variables that are referenced, which should be all we care about for this anyway.

What if the constant is really big? Looks like the importing decision has to be made by the thin linker directly. You can not import the "constant" global as something like "available_externally constant" because the original copy might not be visible from the original module (just like the example), unless you want to modify the original copy to be "external hidden".

Not sure I understand your question, can you clarify the concern? Right now, computeImportForReferencedGlobals during the thin link we import any referenced variable, unless it has interposable linkage or references other summaries (in which case it is not a constant). The imported variables are imported as available externally definitions. The original definition is currently promoted to external hidden.

Nevermind, I didn't realize we are already doing that. I guess we are all fine and this is less complicated than I think it is. Sorry for the noise.

[evgeny777]

I think it would be cleaner/clearer to encode this info along with the Ref edges

This would be a bit of overhead for global to global refs, wouldn't it? We can't make any assumption about them until we traverse an entire index.

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type

What if we have non-static variable? We can't make a local copy of it, but opt pass can still handle this case if there are no external references

Will we miss some cases if the reference is on something like a bitcast that feeds a non-volatile load?

I'll check

[tejohnson]

In D49362#1164028, @evgeny777 wrote:

I think it would be cleaner/clearer to encode this info along with the Ref edges

This would be a bit of overhead for global to global refs, wouldn't it? We can't make any assumption about them until we traverse an entire index.

My understanding of your patch is that there would be a 1-1 association between each bit in the RefAccessBits BitVector and ref edges on that same function. I think my suggestion just reorganizes that slightly by moving the bit onto the ref edge. Does that make sense or am I missing something?

When importing a variable definition when the ref has this bit set, it can be imported as a local copy, without promoting the name/linkage type

What if we have non-static variable? We can't make a local copy of it, but opt pass can still handle this case if there are no external references

If it is non-static to start with, but is globally read-only, then it can be imported as a local copy. In order to do something with an opt pass, don't you also need global analysis during the thin link to ensure there are no writes anywhere?

Will we miss some cases if the reference is on something like a bitcast that feeds a non-volatile load?

I'll check

[evgeny777]

My understanding of your patch is that there would be a 1-1 association between each bit in the RefAccessBits BitVector and ref edges on that same function

Correct. But we have RefEdgeList in a base class (GlobalValueSummary) and there could be references between 2 globals:

static int Foo;
static int *Bar = &Foo;

It doesn't make sense to have const attribute for such kind of references, because we need to know how Bar is accessed before we can say anything about Foo.
That's why I put access bit list to derived class (FunctionSummary). IMO converting std::vector<ValueInfo> refs; to std::vector<EdgeTy> would be a waste of memory
(though not too big).

If it is non-static to start with, but is globally read-only, then it can be imported as a local copy

This also implies internalizing global in the source module, correct?

In order to do something with an opt pass, don't you also need global analysis during the thin link to ensure there are no writes anywhere?

True, but for some reason I thought that global var fixups to BC module would be easier to implement. I'll give "local copy" approach a try and see how it goes.

[tejohnson]

In D49362#1164260, @evgeny777 wrote:

My understanding of your patch is that there would be a 1-1 association between each bit in the RefAccessBits BitVector and ref edges on that same function

Correct. But we have RefEdgeList in a base class (GlobalValueSummary) and there could be references between 2 globals:

static int Foo;
static int *Bar = &Foo;

It doesn't make sense to have const attribute for such kind of references, because we need to know how Bar is accessed before we can say anything about Foo.
That's why I put access bit list to derived class (FunctionSummary).

That's true. Actually, thinking more about the above example - presumably your global analysis on these new const bits would have to look at the references on global variables, and treat that as a non-const ref and basically mark the referenced GUID as non-const, to represent the address as escaping, right? And you will need a way to communicate this info to the backends - will you just clear the const bit on all the referring function summary RefAccessBits?

IMO converting std::vector<ValueInfo> refs; to std::vector<EdgeTy> would be a waste of memory
(though not too big).

Note it would be a different type than EdgeTy which is for calls, but yeah.

If it is non-static to start with, but is globally read-only, then it can be imported as a local copy

This also implies internalizing global in the source module, correct?

To get the optimization in the original source module, yes, but that's only correct if all referencing modules import the def. Which I think is already the case for non-interposable constants.

In order to do something with an opt pass, don't you also need global analysis during the thin link to ensure there are no writes anywhere?

True, but for some reason I thought that global var fixups to BC module would be easier to implement. I'll give "local copy" approach a try and see how it goes.

[evgeny777]

That's true. Actually, thinking more about the above example - presumably your global analysis on these new const bits would have to look at the references on global variables, and treat that as a non-const ref and basically mark the referenced GUID as non-const, to represent the address as escaping, right?

For now I implemented constant propagation in ModuleSummaryIndex which works in three steps:

1. Set Constant = true in all GlobalVarKind GVS and to false in all others
2. Iterate through all live function summaries and propagate Constant = false over each non-constant reference
3. Iterate through all live GlobalVarKind summaries with Constant = false propagate it over all references to other global variables using a work queue.

Also I added few small tweaks to internalize global w/o external usage when Constant is true and produce a local copy during import phase.
I didn't have chance to test it on something real though (just small toy example).

[evgeny777]

I've updated the review with the full version. I've tested on few large applications and results are quite promising in terms of both performance and image size

Will we miss some cases if the reference is on something like a bitcast that feeds a non-volatile load?

I don't think so. Actually this logic conforms to GlobalStatus computation in analyzeGlobalAux, which is the core of globalopt

[evgeny777]

1. Now importing globals list from DICompileUnit in IRMover. This is needed because we may internalise GV in destination module, so it can reach final link.

2. Fixed bug which caused llvm-lto internalising exported symbol with -thinlto-action=import

3. Overall approach was reimplemented in much simpler way

4. Added extra tests.

[evgeny777]

Found few issues with patch

[evgeny777]

Finally I have some time to work on this again
I've simplified a patch to a large extent and tested it on a number of real applications (see below)

Algorithm description

Basically this patch does the following:

• Computes constant references from all functions seen on analysis phase

• Implements constant propagation in combined summary, which allows calculating variables which are never changed in the program

• On the beginning promote phase marks such variables with a special attribute ("thinlto-immutable") in IR if it is possible to internalize them.

• After the end of the import phase internalizes all variables with thinlto-immutable attribute in IR

Key features:

• While computing function references patch postpones processing non-volatile load instructions till all other instructions have been processed. This allows grouping constant refs in the end of RefEdges array and we need only 32 bit unsigned integer value per each function summary to represent all constant references in it.

• Patch can internalize both promoted static variables and variables with external or linkonce linkage. Comdats are not currently supported, but can be done as well.

• BC files after each of the stages (promote/internalize/import e.t.c) are consistent and can be sent to legacy builder for further processing.

Testing

I've tested it on some of real world applications:

LLVM testsuite

Tests pass with following code size improvements/regressions:

Size regressions are caused by more agressive inlining caused by lower inlining costs
due to constant folding. Can provide size analysis data on request

LLVM/clang

LIT tests pass on newly built toolchain. Unfortunately only few variables were internalized,
so no noticable impact was observed

Mozilla Firefox

Produced working image and make check shows all tests passed.

Chromium

Produced working image of both chrome and unit_tests. Some tests fail, but I see no difference
with an image obtained from "regular" build.

Other

I observed 1-2% raise of performance on some of our in-house projects

Missing features

• ThinLTO incremental build *may* fail with link error. To avoid this HasComdat flag has to be added to GlobalValueSummary and used in module hash computation.

• Some Bitcode tests fail due to version bump and extra field in FS_PERMODULE and FS_COMBINED

• Patch lacks tests for various corner cases

This won't take long to fix, but I'd appreciate some feedback for overall approach, while I'm doing this

[tejohnson]

Great! I only had a chance to take a cursory look so far (attending the llvm dev meeting), but a few questions/comments based on a very quick look this morning.

One high level comment is that it would be better to mark the linkage type on the index during the thin link when we can internalize, rather than have that logic in the back end as in this patch:

You'd need to either add the comdat flag to the summary (like you mentioned is needed anyway for incremental builds), or better yet could you just conservatively mark the GlobalVarSummary as non-constant during the compile step module summary analysis (i.e. instead of initializing the Constant bit to false, init it to true for all global variables except those with comdats - then your thin link Step 1 would just set it to false if it's in the GUIDPreservedSymbols set and otherwise not change the flag). This should fix your incremental build issue too.

Then if the thin link step marks those that are constant with internal linkage in the index and applies that in the backend, it would be consistent with the way we do other summary based internalization.

The main thing that I think would need to be changed is in shouldPromoteLocalToGlobal for the importing side we would need to look at the linkage type in the index rather than assuming that anything that is imported must be promoted. (In fact I think that making this change to shouldPromoteLocalToGlobal on the importing side to get the linkage type from the index would work today, since presumably any imported locals are already marked as ExternalLinkage in the index since we currently promote them all.) For the exporting side we already set the linkage type based on what is set in the index, so that would presumably stay the same.

I may be missing something that makes this problematic though, let me know what you think.

A couple other comments below.

include/llvm/IR/ModuleSummaryIndex.h

266

Maybe have this flag just on the GlobalVarSummary? Not needed on the function summary, and presumably the alias summary could just look through to the base object summary.

513

Think it would be clearer to have a ref edge type and include a flag there.

lib/IR/ModuleSummaryIndex.cpp

140

Why? The refs on a global variable are those referenced where it is defined/initialized. Presumably if it is written later it doesn't affect those references.

Hmm, what if we store the address of another static global var into a pointer global variable, and then write through that pointer. How is that case handled by the patch? The load will be to the pointer, so presumably it will not be mutable here. I.e.:

static int A = 1;
static int *B = &A;
void foo() {

*B = 0;

}

In this case, there will be a load of B in foo, so B is constant and not mutable. I might be missing how this is handled, but I don't see where/how A will be marked mutable. I think you might need to be conservative and treat all refs on other global variables as potentially written?

If the address was taken in a function I think you would be ok since it would be referenced by a non-load.

[evgeny777]

In fact I think that making this change to shouldPromoteLocalToGlobal on the importing side to get the linkage type from the index would work today

Yep, this seems obvious and the very first version of this patch uses this kind of approach. Unfortunately it didn't work quite well for me. Modifying linkage in the index will work fine on promotion and
internalization phases. However it's the import phase where all bad things happen. When performing import ThinLTO loads source module and does specific modifications in it given list of globals to import
Those modifications include renaming globals to avoid naming conflicts and setting linkage of imported entities to available_externally. After that IRMover performs all the dirty work.

The main problem in this scheme is handling read only external globals. We can't set internal linkage for them in renameModuleForThinLTO, because IRMover will fail to link variable definition to
it's external declaration. These two variables will be considered different:

@g = internal global i32 0
@g = external global i32

IRMover will rename internal definition, leaving external declaration unsatisfied. This will obviously result in link error.

So while it's possible to use proposed approach to deal with promoted static variables, one still needs some processing in the backend to deal with read only external globals. Still backend approach can deal with both cases, so I used it.

include/llvm/IR/ModuleSummaryIndex.h

266

Makes sense, I'll give it a try

513

What about memory usage? Also, like I said, we have all constant references grouped at the end of RefEdgeList, so this looks excessive

lib/IR/ModuleSummaryIndex.cpp

140

Yep, I also stepped on this recently :). By now I'm considering all non-instruction refs to be non-constant

[evgeny777]

• ReadOnly attribute's been moved to GlobalVarSummary and is calculated on analysis phase
• Added flag to ValueInfo which indicates "constantness" instead of using constant reference counter in FunctionSummary
• Fixed incremental build
• Fixed few issues including GV to GV references
• Fixed bitcode tests and added extra test cases

[tejohnson]

Thanks for the update. A bunch of comments below. I am a little concerned on the relationship between the read only flag on the global var and the importing requirements (see comment in computeVariableSummary). Also, it would be good to have a high level description of the algorithm and how this works somewhere (maybe above propagateConstants()).

In D49362#1268278, @evgeny777 wrote:

In fact I think that making this change to shouldPromoteLocalToGlobal on the importing side to get the linkage type from the index would work today

Yep, this seems obvious and the very first version of this patch uses this kind of approach. Unfortunately it didn't work quite well for me. Modifying linkage in the index will work fine on promotion and
internalization phases. However it's the import phase where all bad things happen. When performing import ThinLTO loads source module and does specific modifications in it given list of globals to import
Those modifications include renaming globals to avoid naming conflicts and setting linkage of imported entities to available_externally. After that IRMover performs all the dirty work.

The main problem in this scheme is handling read only external globals. We can't set internal linkage for them in renameModuleForThinLTO, because IRMover will fail to link variable definition to
it's external declaration. These two variables will be considered different:

@g = internal global i32 0
@g = external global i32

IRMover will rename internal definition, leaving external declaration unsatisfied. This will obviously result in link error.

So while it's possible to use proposed approach to deal with promoted static variables, one still needs some processing in the backend to deal with read only external globals. Still backend approach can deal with both cases, so I used it.

I see, yes that explanation makes sense. I suppose on the importing side we could go back and update the linkage for imported variables based on the linkage in the index after the IRMover importing is complete for the module (since the linkage in the index is currently ignored on the importing side). On the exporting side it would get internalized as usual during the index-based internalization. This has the advantage of being more consistent with how we currently internalize based on thin link analysis. Would that not work as expected?

lib/Analysis/ModuleSummaryAnalysis.cpp

381

Comment needed. I suppose this is because we need to be able to import into any module containing a ref?

[tejohnson]

include/llvm/IR/ModuleSummaryIndex.h

513

I have mixed feelings on this. With your current scheme you don't add increase the in-memory index size, but it is unfortunate to have the read only flag on all ValueInfo when it only applies to reference edges. Let me take a look at some of our large indexes today and see what the extra overhead would be to make this explicit on just the ref edges.

lib/Analysis/ModuleSummaryAnalysis.cpp

419

The initialization below seems to be a combination of conditions on whether it can be internalized and whether it can be imported. Can you better document this?

Non-empty RefEdges isn't related to the notEligibleToImport flag, but rather they both preclude importing of a global var. Don't we also need to set the read only flag based on notEligibleToImport if we want to prevent it from being set unless its def can be imported?

In general, since you presumably need to ensure that all variables marked read only at the end of the thin link are imported into every referencing module, I think it would be good to make this connection more explicit (in comments and in code). Can you enforce this during the thin link (i.e. during computeImportForReferencedGlobals)? E.g. detect when we can't import a reference and then assert if it is marked read only? Alternatively, rather than trying to detect here when it can't be imported and initializing to not read only, during the thin link importing can you clear the read only flag on any variable with a reference that we aren't able to import? Otherwise I am concerned that this relationship may prove fragile.

lib/IR/ModuleSummaryIndex.cpp

36

Is this comment current? It looks like you will look at all of the refs below so I don't see any advantage being taken from them being at the end. You could presumably take advantage of it by breaking out of the loop when you see the first non-readonly ref since you are walking it backwards. Not sure if it is worth it though, might be better just to look at them all for simplicity (and in case this ordering scheme ever changes).

88

Just invoke S->getBaseObject() for simplicity.

95

Does this need to handle S being an alias like the above method? Looks like not from the current callsite, but I think it should invoke getBaseObject() to be consistent with the above method and also to be safe in case new calls are added that might pass a generic GlobalValueSummary.

109

Is there a need to perform each of the steps below in a separate walk over the whole index? I don't see that we actually propagate the flag from global var to global var, so I don't see any need for ordering. Can you do one walk over the index and perform all steps on each summary exactly once?

117

This first sentence doesn't seem to relate specifically to Step 1, but rather to the whole algorithm.

121

I would expect the size of this set to be << the size of the whole index, so walking the whole index and checking each seems like it might be less efficient overall compared to doing a lookup in the index of each GUID preserved symbol. That being said, I think a better way is to walk the index once and perform all 3 steps (as mentioned above).

132

Suggest adding the explanation of why.

lib/LTO/LTO.cpp

160

I don't think this is necessary because later on (line 221) we walk all the import summaries and invoke AddUsedThings on them.

lib/Transforms/IPO/FunctionImport.cpp

715

Why is this necessary? Since internalizeImmutableGVs is only invoked from importFunctions(), presumably it doesn't have any effect anyway when not importing?

[evgeny777]

I see, yes that explanation makes sense. I suppose on the importing side we could go back and update the linkage for imported variables based on the linkage in the index after the IRMover importing is complete for the module (since the linkage in the index is currently ignored on the importing side). On the exporting side it would get internalized as usual during the index-based internalization. This has the advantage of being more consistent with how we currently internalize based on thin link analysis. Would that not work as expected?

It seems to me as significantly more complex approach. At the first glance, I'll have to:

• Prevent promotion on thin link phase in both LTO.cpp and ThinLTOCodeGenerator.cpp for all read-only variables.
• On promote phase check all read-only variables with local linkage preventing them from becoming available_externally. Non-local vars must not be touched (or IRMover will fail to link them to declarations), so we also need step 3:
• After import phase we still need to analyze external read only globals, internalizing them when possible. As I'm supposed to check index instead of IR attributes I'm puzzled how to do this properly because of changing linkage to available_externally during import process (so GUID will obviously change also)

So compared to current implementation we:

• add extra step (prevent promotion in index) and make handling of external globals signifcantly more complex.
• must have all three steps consistent, which means spreading checks around the code. For instance, if variable is not eligible to import it (a) shouldn't be prevented from promotion, (b) should have available_externally linkage when being imported and (c) shouldn't be internalized if it has non-local linkage.

include/llvm/IR/ModuleSummaryIndex.h

513

I've already changed the way immutable references are stored. Now I'm using an extra flag in ValueInfo which supposedly has zero overhead. I'm still using immutable reference counter when store/load index to/from bitcode file as all read-only references are still grouped in the end of RefEdges array to save disk space.

lib/Analysis/ModuleSummaryAnalysis.cpp

381

We can't internalize a variable if it is referenced by a function in regular LTO module. This is because regular LTO modules are not participating in ThinLTO import, so we can't make a local copy there.

419

Makes sense.

The RefEdges.empty() is the only condition on whether a variable can be imported, all others are related to internalization. I think, I can safely move it (and only it) to propagateConstants if it looks confusing here. My concerns were ThinLTO cache key computation, but it seems such move won't affect it , because import and export lists will be different so will be the cache key.

lib/IR/ModuleSummaryIndex.cpp

36

Yes, the comment is current, but there is a bug in implementation, it should be something like:

for (int I = Refs.size() - 1; I >= 0 && Refs[I].isReadOnly(); --I)
    ImmutableRefCnt ++;

95

This is a helper method used by dot dumper exclusively. I think it should be moved below to a place of actual call.

lib/LTO/LTO.cpp

160

Yep, I missed second call of AddUsedThings

lib/Transforms/IPO/FunctionImport.cpp

715

On -lto-O0 ThinLTO import is entirely disabled, so we have to prevent internalization of read-only stuff or we'll get link errors.

[tejohnson]

Just a quick comment below since I'm traveling today and out tomorrow and won't have time for a better response until probably Monday. You may be right on the issue of using the index vs just using the read only bit, need to look more closely at your explanation.

lib/Transforms/IPO/FunctionImport.cpp

715

Right but since the internalization is done via internalizeImmutableGVs which is called from importFunctions, if importing is disabled under -lto-O0 wouldn't the internalization not be done either?

[evgeny777]

lib/Transforms/IPO/FunctionImport.cpp

715

Well, strictly speaking, import in the backend is not disabled: -lto-O0 just prevents computation of import/export lists, leaving them empty. Current approach marks read-only GVs with thinlto-internalize attribute, relying on IRMover to copy them to all destination modules, so internalization happens in all modules using read-only GV (or shouldn't happen at all). If IRMover doesn't do anything (because import lists are empty) we'll internalize GV just in the source module, leaving all external declarations unsatisfied.

[tejohnson]

In D49362#1275404, @evgeny777 wrote:

I see, yes that explanation makes sense. I suppose on the importing side we could go back and update the linkage for imported variables based on the linkage in the index after the IRMover importing is complete for the module (since the linkage in the index is currently ignored on the importing side). On the exporting side it would get internalized as usual during the index-based internalization. This has the advantage of being more consistent with how we currently internalize based on thin link analysis. Would that not work as expected?

It seems to me as significantly more complex approach. At the first glance, I'll have to:

• Prevent promotion on thin link phase in both LTO.cpp and ThinLTOCodeGenerator.cpp for all read-only variables.
• On promote phase check all read-only variables with local linkage preventing them from becoming available_externally. Non-local vars must not be touched (or IRMover will fail to link them to declarations), so we also need step 3:
• After import phase we still need to analyze external read only globals, internalizing them when possible. As I'm supposed to check index instead of IR attributes I'm puzzled how to do this properly because of changing linkage to available_externally during import process (so GUID will obviously change also)

So compared to current implementation we:

• add extra step (prevent promotion in index) and make handling of external globals signifcantly more complex.
• must have all three steps consistent, which means spreading checks around the code. For instance, if variable is not eligible to import it (a) shouldn't be prevented from promotion, (b) should have available_externally linkage when being imported and (c) shouldn't be internalized if it has non-local linkage.

Ok, this does seem substantially simpler, so let's go ahead with your current approach. I made a few more comments below, will take another look after you've had a chance to update.

include/llvm/IR/ModuleSummaryIndex.h

513

ok let's go with this approach. If we need to add more info on the ref edges in the future, it can be revisited.

lib/LTO/ThinLTOCodeGenerator.cpp

649

document constant parameter.

lib/Transforms/IPO/FunctionImport.cpp

715

Ah ok, I missed that lto-O0 was just skipping the index based computation of imports and not the backend importFunctions invocation.

lib/Transforms/Utils/FunctionImportUtils.cpp

228 ↗

(On Diff #170611)

Please expand on why the internalization of the read only variables needs to be done this way (i.e. in two steps).

230 ↗

(On Diff #170611)

This adds another index hash table lookup for GV, which we do just above here too. Can you lookup the ValueInfo for the GV once for both blocks of code?

231 ↗

(On Diff #170611)

We should check notEligibleToImport during the thin link, along the lines of my comment in computeVariableSummary. Also, why is the isLive check needed?

[evgeny777]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

We should check notEligibleToImport during the thin link, along the lines of my comment in computeVariableSummary

This is problematic because notEligibleToImport is calculated after computing per-module summaries. How about doing this in propagateConstants (as we're going to switch to single index pass algorithm anyway)?

Also, why is the isLive check needed?

To prevent internalization of non-prevailing external globals. See LTO/Resolution/X86/not-prevailing-variables.ll

[tejohnson]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

We should check notEligibleToImport during the thin link, along the lines of my comment in computeVariableSummary

This is problematic because notEligibleToImport is calculated after computing per-module summaries. How about doing this in propagateConstants (as we're going to switch to single index pass algorithm anyway)?

Right, note I suggested doing it in the thin link. Similar to my comment in computeVariableSummary where I suggested checking the import eligibility during the thin link: "Alternatively, rather than trying to detect here when it can't be imported and initializing to not read only, during the thin link importing can you clear the read only flag on any variable with a reference that we aren't able to import? "

Also, why is the isLive check needed?

To prevent internalization of non-prevailing external globals. See LTO/Resolution/X86/not-prevailing-variables.ll

Won't we eliminate these non-live variables in any case (regardless of whether we internalize)? Or will internalization mess that up? In any case, it's probably better if this is also checked during the thin link (e.g. propagateConstants), and just clear the read only flag if we shouldn't internalize in the backend.

[evgeny777]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

it's probably better if this is also checked during the thin link

We should be able to handle case when alias is dead and aliasee is live (and vice versa). I don't see how this is possible to do with just manipulating ReadOnly attribute because we don't have it in AliasSummary.

[tejohnson]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

This case is only handling global variables (getGVarSummary looks only for a summary of that type, and there is a cast below), so I'm not sure I understand how this part of the code relates to the case where the alias is dead and the aliasee is live. Can you clarify?

If the aliasee (the GVar) is dead, it seems like we could clear the read only flag during the thin link and get the same behavior as today.

Sorry if I am missing something. Can you give me a more concrete example of where/how this would go wrong for aliases?

[evgeny777]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

Consider the following example:

foo.ll

@g = global i32 42, align 4
@g.alias = weak alias i32, i32* @g

main.ll

@g = external global i32

define i32 @main() {
  %v = load i32, i32* @g
  ret i32 %v
}

Assume we link main.ll and foo.ll in ThinLTO mode. We have global variable @g and it's alias @g.alias, the latter is obviously dead. Still it doesn't mean we can't internalize aliasee (@g), does it?

[tejohnson]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

Assume we link main.ll and foo.ll in ThinLTO mode. We have global variable @g and it's alias @g.alias, the latter is obviously dead. Still it doesn't mean we can't internalize aliasee (@g), does it?

I'm confused though at how this relates to my suggestion. Here you are checking whether the global variable, i.e. @g, is live or not. In this example it is live, so presumably you will go ahead and mark it for internalization here since it is also read only. If you move the liveness check into the thin link (propagateConstants), presumably the same thing would happen - it is live, so it can stay read only, and this code would still mark it for internalization.

[evgeny777]

lib/Transforms/Utils/FunctionImportUtils.cpp

231 ↗

(On Diff #170611)

Ah, got it now. Ok, I'll try this out

[evgeny777]

• Addressed review comments
• Added call to propagateConstants to createCombinedModuleSummaryIndex to support -thinlto mode of llvm-lto

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

382

suggest adding something like "since this would require importing variable as local copy"

lib/IR/ModuleSummaryIndex.cpp

95

Where is this being handled? I don't see any special handling of references from GlobalVars here.

138

Should this and the above block of code (looking for live values) only be done if S is a GlobalVariableSummary?

Also, can the conditions for importing be shared with computeImportForReferencedGlobals so that the conditions don't diverge? I.e. some kind of helper method to do the checking. Also I noticed that computeImportForReferencedGlobals also checks whether it is interposable.

tools/llvm-lto/llvm-lto.cpp

389 ↗

(On Diff #171940)

Why this change? IIRC this is just to test the creation of the combined index, with no optimizations done on it. Can we test the constant propagation elsewhere where the various thinlto optimizations are tested (e.g. see the ThinLTOMode handling).

[evgeny777]

lib/IR/ModuleSummaryIndex.cpp

95

In ValueInfo constructor we have:

RefAndFlags.setInt(HaveGVs);

This means that ReadOnly bit is zero, unless setReadOnly is called

138

Should this and the above block of code (looking for live values) only be done if S is a GlobalVariableSummary?

If alias is either not eligible to import or is preserved we can't make a local copy of aliasee, can we? If we make this check only for GlobalVarSummary we can internalize it if its alias is preserved

Also, can the conditions for importing be shared with computeImportForReferencedGlobals so that the conditions don't diverge?

Sounds reasonable.

Also I noticed that computeImportForReferencedGlobals also checks whether it is interposable.

We don't set ReadOnly bit for GVs with interposable linkage. See computeVariableSummary

tools/llvm-lto/llvm-lto.cpp

389 ↗

(On Diff #171940)

Unless this is done the patch breaks Linker/funcimport.ll test case due to internalization of dead GV. I'm not quite aware of the purpose of this code and how to fix it properly. Suggestions?

[tejohnson]

lib/IR/ModuleSummaryIndex.cpp

95

Can you update the comment here to note that this is done by only marking refs from functions as read only when building the module summary during the analysis phase? Also suggest adding some checking here under NDEBUG that if VI is read only that S is not a global var summary. Will help prevent drift between the comments here and any changes to the module summary analysis phase later.

138

If alias is either not eligible to import or is preserved we can't make a local copy of aliasee, can we? If we make this check only for GlobalVarSummary we can internalize it if its alias is preserved

I'm not sure why we couldn't import and make a local copy of the aliasee when an alias to it is preserved and/or not eligible to import - assuming the reference is to the aliasee not the alias. In fact, we currently will never import a reference to an alias of a global var AFAICT [1].

[1] computeImportForReferencedGlobals only handles refs from functions directly to GlobalVarSummary objects - it doesn't call getBaseObject on the refs. So I don't believe we will even consider importing an alias to a global var. If that is ever supported in the future, we would presumably want to handle it like we do importing of an alias to a function, which is to import as a local copy [2]. In that case, we'd presumably want to ensure that the alias was importable, that the aliasee is importable, and that the aliasee is read only. However I don't think that requires marking the aliasee as not read only if the alias is not importable.

[1] As an aside, looking at function importing, it will look through to the base object (in selectCallee) *before* checking the notEligibleToImport flag. So I don't think having an alias marked as notEligibleToImport will actually prevent it from being imported! I think this works currently since AFAICT the only time we will mark an alias summary as notEligibleToImport is when it is a local that can't be promoted/renamed, and we only import aliases to functions as a local copy, which doesn't require promotion/renaming of the alias in the original module. This seems a little dicey to me, if we ever decide to mark an alias summary as notEligibleToImport for some other reason it might cause failures...not related at all to this patch, just dumping my thoughts here as a note to self...

We don't set ReadOnly bit for GVs with interposable linkage. See computeVariableSummary

Ok. In any case, extracting the importability check into a helper called both here and in the importer will keep these checks consistent (I'd prefer the interposability to be checked redundantly over having the checks for importability be different).

tools/llvm-lto/llvm-lto.cpp

389 ↗

(On Diff #171940)

Ah, that test uses some legacy functionality in llvm-link to test importing. We've already had to work around the fact that it isn't doing a true thin link before importing, with a hack to mark all variables as exported (in linkFiles). To fix this you should be able to move this call to propagateConstants() onto the index after we load it in importFunctions (in llvm-link.cpp).

The code here is just building the original combined index which in most cases is used to drive thin link optimizations via llvm-lto. Probably these old tests that use llvm-link to test importing should be migrated to llvm-lto and the import handling in llvm-link should be ripped out. I'll try to get to this today or tomorrow, but in the meantime, you should be able to work around in llvm-link itself as noted above.

[evgeny777]

I'm not sure why we couldn't import and make a local copy of the aliasee when an alias to it is preserved and/or not eligible to import

My guess is because alias and aliasee are in the fact the same object (both point to the same memory location). Now assume alias is preserved and visible outside of the DSO. One can modify the object via alias (during program initialization for example) which
will take no effect on aliasee if it's been imported to a different module and internalized. The same applies when alias is listed in llvm.used

[tejohnson]

In D49362#1289042, @evgeny777 wrote:

I'm not sure why we couldn't import and make a local copy of the aliasee when an alias to it is preserved and/or not eligible to import

My guess is because alias and aliasee are in the fact the same object (both point to the same memory location). Now assume alias is preserved and visible outside of the DSO. One can modify the object via alias (during program initialization for example) which
will take no effect on aliasee if it's been imported to a different module and internalized. The same applies when alias is listed in llvm.used

Ok, that makes sense. For normal importing, we don't need to look at the importability of the alias on a reference to the aliasee since we will promote it. The difference here is that these conditions on the alias mean we don't have visibility into possible writes to the aliasee via the alias, and thus can't safely mark it read only. Can you add a comment to this effect in propagateConstants where you are checking those conditions?

I think it would be clearer in propagateConstants to do the checks only on the applicable summary types:

1. liveness check only on global var summaries
2. the importability checks on either global var or alias summaries (with note as suggested above about why on alias summaries)
3. the call to propagateConstantsToRefs only on function summaries

It works without the checks, but I think that guarding with the appropriate checks + a comment as to why would aid in understanding.

[evgeny777]

the call to propagateConstantsToRefs only on function summaries

This will not work, because we need to drop read only attribute from GVs, referenced by initializer

[tejohnson]

In D49362#1289304, @evgeny777 wrote:

the call to propagateConstantsToRefs only on function summaries

This will not work, because we need to drop read only attribute from GVs, referenced by initializer

Right, forgot about the fact that while the GlobalVar refs are never marked read only, the info still needs to be propagated. So might as well do that call on all summaries, since you already note in the function that for aliases it will be a no-op.

[tejohnson]

tools/llvm-lto/llvm-lto.cpp

389 ↗

(On Diff #171940)

I tried migrating the llvm-link tests to llvm-lto but it turns out not to be straightforward for a couple of tests that are testing importing in the presence of old debug info, because we don't have a way other than with llvm-link to force the importing of a given function, and the bitcode being old means the index is old and we don't have importing flags set so importing is conservative. So I suggest going with the fix I suggested above for now.

[evgeny777]

Rebased and addressed review comments

[tejohnson]

Looks really good - just one minor code change request, and a few comment changes, and one question.

lib/IR/ModuleSummaryIndex.cpp

133

Please update the comment to say why they can't be internalized. We drop dead symbols anyway in the backend, but I guess the issue is that once it is internalized the GUID changes and we can't find the summary in dropDeadSymbols?

134

If/when we start importing aliases to global vars, would that affect this code? I still think we wouldn't want to update the readonly bit on the aliasee if alias is dead. (We shouldn't do any importing of dead symbols in any case - only live roots are added to the import worklist.)

138

typo: s/referneces/references/

144

Comment is still a little vague. Specifically, from what I understand:

• global variable can't be marked read only if it is not eligible to import since we need to ensure that all external references get a local (imported) copy.
• a global variable can't be marked read only if it or any alias (since alias points to the same memory) are preserved or notEligibleToImport, since either of those means there could be writes that are not visible (because preserved means it could have external to DSO writes, and notEligibleToImport means it could have writes via inline assembly leading it to be in the @llvm.*used).

Can you update the comment to note the above specifics (adjusted if I am missing something)? Will avoid future head scratching when I or someone else looks at this again down the road.

lib/Transforms/IPO/FunctionImport.cpp

263

Please move this one into canImportGlobalVar. I'd rather check it redundantly in the readonly case (i.e. even though you are already checking when building the summary and initializing that bit), to keep the checking centralized and consistent between here and there.

[evgeny777]

lib/IR/ModuleSummaryIndex.cpp

133

This was originally intended to prevent internalization of non-prevailing defs. See test/LTO/Resolution/X86/not-prevailing-variables.ll

134

If/when we start importing aliases to global vars, would that affect this code

I would simply move liveness check to processGlobalsForThinLTO where it originally was. This will also allow eliminating the calls to propagateConstants from llvm-link.cpp. Of course computeImportForReferencedGlobals must be modified to support aliases

144

Can you update the comment to note the above specifics

Can I simply quote you in comments (as your understanding is 100% correct)? :)

[tejohnson]

lib/IR/ModuleSummaryIndex.cpp

133

Right but I'm trying to get to the *why* of that no longer working if you didn't do the liveness check here. As noted in that test case, we should drop the definition of that non-prevailing var2. I'm speculating that this would stop working if you marked var2 as readOnly since it would get internalized which would mean that dropDeadSymbols would compute a different GUID for the now-internalized variable which in turn would mean that we can't find the summary to see that it is dead.

134

But I don't understand why that would require a change to any of this handling? I.e. if an alias is dead, we should never even encounter it in computeImportForReferencedGlobals, since we only compute importing starting from live roots, and if the alias is reached from a live root then by definition it can't be dead.

144

Sure =)

[evgeny777]

lib/IR/ModuleSummaryIndex.cpp

133

Well, here is what I find out:

• dropDeadSymbols is invoked right after promotion and before internalization of immutable GVs, which happens much later (after import is finished). I rechecked not-prevailing-variables.ll test case and it works perfectly, even if liveness check is removed. Small modification to internalizeImmutableGVs is still required: we need to ignore declarations, not assert on them.

• If we remove liveness check then funcimport.ll test case will fail. This happens because dropDeadSymbols checks for ModuleSummaryIndex::isGlobalValueLive which can return true even if isLive() returns false. In particular this happens when all symbols in the index are dead.

That said I suggest to:

• remove liveness check from propagateConstants
• revert changes to llvm-link.cpp
• update the failed test cases

134

If we remove liveness check (see above) then, I guess, we can simply add required functionality to computeImportForReferencedGlobals

[evgeny777]

• Addressed review comments
• Reverted changes in llvm-link.cpp. See inline comments
• No longer clearing read only attribute on dead variables in propagateConstants. Liveness check still remains because we must ignore dead values during constant propagation. As a result the following tests were updated

test/Linker/funcimport.ll
test/Transforms/FunctionImport/funcimport.ll

• Changed isLive() to isGlobalValueLive() in propagateConstants

[tejohnson]

LGTM - just a couple of comments in the modified tests that need updates before commit. Thanks!

test/Linker/funcimport.ll

14 ↗

(On Diff #173149)

Update comment.

17 ↗

(On Diff #173149)

Update this comment - "Eventually" is now! =)

test/Transforms/FunctionImport/funcimport.ll

83 ↗

(On Diff #173149)

update comment.

[evgeny777]

Sorry I have to suggest small amendment to this patch before committing after looking test/ThinLTO/X86/distributed_import.ll.
It looks like distributed import is using per-module indexes, so internalization of RO vars will not work correctly. To overcome this
I suggest adding a check for propagateConstants to have been run before marking GV for internalization

See https://reviews.llvm.org/D54306

This change causes modifications to FunctionImport/funcimport.ll to be reverted. The check for withGlobalValueDeadStripping is a bit counter intuitive, so suggestions welcome.

[tejohnson]

In D49362#1292750, @evgeny777 wrote:

Sorry I have to suggest small amendment to this patch before committing after looking test/ThinLTO/X86/distributed_import.ll.
It looks like distributed import is using per-module indexes, so internalization of RO vars will not work correctly. To overcome this
I suggest adding a check for propagateConstants to have been run before marking GV for internalization

See https://reviews.llvm.org/D54306

This change causes modifications to FunctionImport/funcimport.ll to be reverted. The check for withGlobalValueDeadStripping is a bit counter intuitive, so suggestions welcome.

Why won't it work for distributed builds? The read only gvar flag is being serialized out and back in, so shouldn't it show up in the distributed indexes? If not, the problem should be fixed there so it can work with distributed builds, rather than skipping it (otherwise e.g. here at Google we won't be able to take advantage of this).

[evgeny777]

Why won't it work for distributed builds? The read only gvar flag is being serialized out and back in, so shouldn't it show up in the distributed indexes?

It does, but we can't be sure about "read-only" unless we analyze function references. And we need propagateConstants for that.

[tejohnson]

In D49362#1292863, @evgeny777 wrote:

Why won't it work for distributed builds? The read only gvar flag is being serialized out and back in, so shouldn't it show up in the distributed indexes?

It does, but we can't be sure about "read-only" unless we analyze function references. And we need propagateConstants for that.

propagateConstants should run during the thin link, which would happen in a distributed build before writing indexes. Maybe there is something weird about that test where it's not actually running the real thin link, will look...

[tejohnson]

In D49362#1292864, @tejohnson wrote:

In D49362#1292863, @evgeny777 wrote:

Why won't it work for distributed builds? The read only gvar flag is being serialized out and back in, so shouldn't it show up in the distributed indexes?

It does, but we can't be sure about "read-only" unless we analyze function references. And we need propagateConstants for that.

propagateConstants should run during the thin link, which would happen in a distributed build before writing indexes. Maybe there is something weird about that test where it's not actually running the real thin link, will look...

Hmm that test is using llvm-lto2 to run the thin link and write distributed indexes, which should do the full thin link. Can you look at why it isn't running propagateConstants during the thin link?

That being said, your change in D54306 seems fine in that we wouldn't have run propagateConstants if the dead stripping bit in the index isn't set. But there again, we should have run dead stripping and therefore propagateConstants during the thin link of llvm-lto2, set the with dead stripping bit in the index, and serialized that through the distributed indexes. Can you see why that isn't being run and set in the distributed indexes (both the with dead stripping flag and the read only flags)?

[evgeny777]

Oh, my bad. opt -function-import gets index from llvm-lto2, not from opt -thinlto-bc. Sorry about that

However, there is still a small problem:

In FunctionImport/funcimport.ll we use index from llvm-lto -thinlto, for which propagateConstants hasn't been run. This results in internalizing P.llvm.0, which shouldn't happen, because it's writeable (from setfunc)
Can such thing occur in real life (i.e we use index from llvm-lto -thinlto in distrivuted build)?

[tejohnson]

In D49362#1292907, @evgeny777 wrote:

Oh, my bad. opt -function-import gets index from llvm-lto2, not from opt -thinlto-bc. Sorry about that

However, there is still a small problem:

In FunctionImport/funcimport.ll we use index from llvm-lto -thinlto, for which propagateConstants hasn't been run. This results in internalizing P.llvm.0, which shouldn't happen, because it's writeable (from setfunc)

Ok that makes sense.

Can such thing occur in real life (i.e we use index from llvm-lto -thinlto in distrivuted build)?

Nope, that's just a testing config. So your follow on fix seems fine to fix that.

[steven_wu]

This commit causes our internal bots failed to bootstrap clang. The error we are getting is:

"gCrashRecoveryEnabled (.llvm.1401930837577591816)", referenced from:
    clang::ParseAST(clang::Sema&, bool, bool) in 286.thinlto.o

I have a bit trouble to reproduce the problem but it is 100% reproducible on our bots. I suspect the issue is ThinLTO cache reuse because if I clear the cache then the link will succeed. I will update if I can reproduce and pinpoint the issue.

[steven_wu]

I reverted this in r346768.

I think this is indeed a caching problem. There are some dylib/binary in clang projects that toggles whether to enable the recovery context but some does not. I can reproduce the issue by thin link libclang.dylib first, then link clang-func-mapping on Darwin.
libclang.dylib thinks the file scope variable gCrashRecoveryEnabled not read-only, so it promotes it.
clang-func-mapping thinks gCrashRecoveryEnabled read-only, so it internalize and constant propagate the variable but ParseAST.o in the cache is still expecting gCrashRecoveryEnabled to be available.

Let me know if you need more information.

[tejohnson]

In D49362#1297188, @steven_wu wrote:

I reverted this in r346768.

I think this is indeed a caching problem. There are some dylib/binary in clang projects that toggles whether to enable the recovery context but some does not. I can reproduce the issue by thin link libclang.dylib first, then link clang-func-mapping on Darwin.
libclang.dylib thinks the file scope variable gCrashRecoveryEnabled not read-only, so it promotes it.
clang-func-mapping thinks gCrashRecoveryEnabled read-only, so it internalize and constant propagate the variable but ParseAST.o in the cache is still expecting gCrashRecoveryEnabled to be available.

Let me know if you need more information.

Thinking through this example, I'm not sure what is going on. This patch did change the cache key computation to include the read only bit from the global var summary of anything defined or imported. Since gCrashRecoveryEnabled is a static in CrashRecoveryContext.cpp, it must have been imported into ParseAST.o, which means that the read only bit on the associated gvar summary should have been hashed into ParseAST.o's cache key. So presumably we shouldn't have had a cache hit for ParseAST.o (built when the read only bit is not set on gCrashRecoveryEnabled) when thin linking clang-func-mapping which has this bit set for that variable.

Even if the variable was originally externally visible, in order for it to be marked read only by the thin link it would have had to have been imported at all use sites. Which means that any referencing module had to have it in the import set (and therefore would have hashed the read only bit) in the thin link where it was marked read only. So I am not sure offhand why we could ever share a referencing object between a link where it was read only and another link where it is not... Hopefully Eugene can figure out what is going wrong here.

[tejohnson]

I was trying this out internally and noticed a couple of minor things that can be fixed when you recommit the patch with the caching fix.

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

1048 ↗

(On Diff #173495)

This should always be true - globals() returns only GlobalVariables (you shouldn't even need to cast).

llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp

225 ↗

(On Diff #173495)

As we discussed earlier in the review thread, there should not be any issue with doing this for a distributed import (I just checked a small test case and confirmed it works fine). Please update the comment (it was only in certain testing contexts that you wouldn't have dead stripping at this point).

include/llvm/Bitcode/LLVMBitCodes.h

enum GlobalValueSummarySymtabCodes {
   FS_PERMODULE_RELBF = 19,
   // Index-wide flags
   FS_FLAGS = 20,
   // Maps type identifier to summary information for that type identifier.
   // TYPE_ID: [typeid, kind, bitwidth, align, size, bitmask, inlinebits,
   //           n x (typeid, kind, name, numrba,
   //                numrba x (numarg, numarg x arg, kind, info, byte, bit))]
   FS_TYPE_ID = 21,
+  // List of function constant references
+  FS_CONST_REF_LIST = 22,
 };
 
 enum MetadataCodes {
   METADATA_STRING_OLD = 1,     // MDSTRING:      [values]
   METADATA_VALUE = 2,          // VALUE:         [type num, value num]
   METADATA_NODE = 3,           // NODE:          [n x md num]
   METADATA_NAME = 4,           // STRING:        [values]
   METADATA_DISTINCT_NODE = 5,  // DISTINCT_NODE: [n x md num]

include/llvm/IR/ModuleSummaryIndex.h

 ///  hold the module index and summary for function importing.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_MODULESUMMARYINDEX_H
 #define LLVM_IR_MODULESUMMARYINDEX_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Module.h"
struct GVFlags {
     ///
     /// In combined summary, indicate that the global value is live.
     unsigned Live : 1;
 
     /// Indicates that the linker resolved the symbol to a definition from
     /// within the same linkage unit.
     unsigned DSOLocal : 1;
 
+    /// Indicates that global isn't modified by any live function
+    unsigned Constant : 1;
+
     /// Convenience Constructors
     explicit GVFlags(GlobalValue::LinkageTypes Linkage,
                      bool NotEligibleToImport, bool Live, bool IsLocal)
         : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport),
-          Live(Live), DSOLocal(IsLocal) {}
+          Live(Live), DSOLocal(IsLocal), Constant(false) {}
   };
 
 private:
   /// Kind of summary for use in dyn_cast<> et al.
   SummaryKind Kind;
 
   GVFlags Flags;
 
public:
   bool isLive() const { return Flags.Live; }
 
   void setLive(bool Live) { Flags.Live = Live; }
 
   void setDSOLocal(bool Local) { Flags.DSOLocal = Local; }
 
   bool isDSOLocal() const { return Flags.DSOLocal; }
 
+  void setConstant(bool Constant) { Flags.Constant = Constant; }
+
+  bool isConstant() const { return Flags.Constant; }
+
   /// Flag that this global value cannot be imported.
   void setNotEligibleToImport() { Flags.NotEligibleToImport = true; }
 
   /// Return the list of values referenced by this global value definition.
   ArrayRef<ValueInfo> refs() const { return RefEdgeList; }
 
   /// If this is an alias summary, returns the summary of the aliased object (a
   /// global variable or function), otherwise returns itself.
private:
 
   /// Function attribute flags. Used to track if a function accesses memory,
   /// recurses or aliases.
   FFlags FunFlags;
 
   /// List of <CalleeValueInfo, CalleeInfo> call edge pairs from this function.
   std::vector<EdgeTy> CallGraphEdgeList;
 
+  /// Bit vector, where each bit corresponds to a single outgoing reference.
+  /// Value of 1 means read-only access to referenced valuec.
+  BitVector RefAccessBits;
+
   std::unique_ptr<TypeIdInfo> TIdInfo;
 
 public:
   FunctionSummary(GVFlags Flags, unsigned NumInsts, FFlags FunFlags,
                   std::vector<ValueInfo> Refs, std::vector<EdgeTy> CGEdges,
                   std::vector<GlobalValue::GUID> TypeTests,
                   std::vector<VFuncId> TypeTestAssumeVCalls,
                   std::vector<VFuncId> TypeCheckedLoadVCalls,
                   std::vector<ConstVCall> TypeTestAssumeConstVCalls,
                   std::vector<ConstVCall> TypeCheckedLoadConstVCalls)
       : GlobalValueSummary(FunctionKind, Flags, std::move(Refs)),
         InstCount(NumInsts), FunFlags(FunFlags),
         CallGraphEdgeList(std::move(CGEdges)) {
     if (!TypeTests.empty() || !TypeTestAssumeVCalls.empty() ||
         !TypeCheckedLoadVCalls.empty() || !TypeTestAssumeConstVCalls.empty() ||
         !TypeCheckedLoadConstVCalls.empty())
       TIdInfo = llvm::make_unique<TypeIdInfo>(TypeIdInfo{
           std::move(TypeTests), std::move(TypeTestAssumeVCalls),
           std::move(TypeCheckedLoadVCalls),
           std::move(TypeTestAssumeConstVCalls),
           std::move(TypeCheckedLoadConstVCalls)});
   }
 
+  const BitVector &getRefAccessBits() const { return RefAccessBits; }
+  void setRefAccessBits(BitVector &&BV) {
+    // We accept either empty bit vector (means all refs are RW) or
+    // one of size equal to number of references.
+    assert(BV.empty() || BV.size() == refs().size());
+    RefAccessBits = std::move(BV);
+  }
+
   /// Check if this is a function summary.
   static bool classof(const GlobalValueSummary *GVS) {
     return GVS->getSummaryKind() == FunctionKind;
   }
 
   /// Get function attribute flags.
   FFlags fflags() const { return FunFlags; }
 
private:
   friend yaml::MappingTraits<ModuleSummaryIndex>;
 
   GlobalValueSummaryMapTy::value_type *
   getOrInsertValuePtr(GlobalValue::GUID GUID) {
     return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo(HaveGVs))
                  .first;
   }
 
+  void propagateConstantsForFunction(FunctionSummary *FS);
+  void collectMutableGVSummaries(GlobalValueSummary *Root,
+                                 DenseSet<GlobalValueSummary *> &MutableGVSet);
+
 public:
   // See HaveGVs variable comment.
   ModuleSummaryIndex(bool HaveGVs) : HaveGVs(HaveGVs), Saver(Alloc) {}
 
   bool haveGVs() const { return HaveGVs; }
 
   gvsummary_iterator begin() { return GlobalValueMap.begin(); }
   const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
public:
   /// Dump to stderr (for debugging).
   void dump() const;
 
   /// Export summary to dot file for GraphViz.
   void exportToDot(raw_ostream& OS) const;
 
   /// Print out strongly connected components for debugging.
   void dumpSCCs(raw_ostream &OS);
+
+  /// Analyze index and detect unmodified globals
+  void propagateConstants(const DenseSet<GlobalValue::GUID> &PreservedSymbols);
 };
 
 /// GraphTraits definition to build SCC for the index
 template <> struct GraphTraits<ValueInfo> {
   typedef ValueInfo NodeRef;
 
   static NodeRef valueInfoFromEdge(FunctionSummary::EdgeTy &P) {
     return P.first;

include/llvm/Linker/IRMover.h

public:
   ///
   /// - \p AddLazyFor is a call back that the IRMover will call when a global
   ///   value is referenced by one of the ValuesToLink (transitively) but was
   ///   not present in ValuesToLink. The GlobalValue and a ValueAdder callback
   ///   are passed as an argument, and the callback is expected to be called
   ///   if the GlobalValue needs to be added to the \p ValuesToLink and linked.
   /// - \p IsPerformingImport is true when this IR link is to perform ThinLTO
   ///   function importing from Src.
-  Error move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
-             std::function<void(GlobalValue &GV, ValueAdder Add)> AddLazyFor,
-             bool IsPerformingImport);
+  Error
+  move(std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
+       std::function<void(GlobalValue *DGV, GlobalValue *SGV)> MaterializerHook,
   Module &getModule() { return Composite; }
 
 private:
   Module &Composite;
   IdentifiedStructTypeSet IdentifiedStructTypes;
   MDMapT SharedMDs; ///< A Metadata map to use for all calls to \a move().
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/Transforms/IPO/FunctionImport.h

 /// \p GUIDPreservedSymbols. Non-prevailing symbols are symbols without a
 /// prevailing copy anywhere in IR and are normally dead, \p isPrevailing
 /// predicate returns status of symbol.
 void computeDeadSymbols(
     ModuleSummaryIndex &Index,
     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
     function_ref<PrevailingType(GlobalValue::GUID)> isPrevailing);
 
+/// Compute dead symbols and run constant propagation in combined index
+/// after that.
+void computeDeadSymbolsWithConstProp(
+    ModuleSummaryIndex &Index,
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
+    function_ref<PrevailingType(GlobalValue::GUID)> isPrevailing);
+
 /// Converts value \p GV to declaration, or replaces with a declaration if
 /// it is an alias. Returns true if converted, false if replaced.
 bool convertToDeclaration(GlobalValue &GV);
 
 /// Compute the set of summaries needed for a ThinLTO backend compilation of
 /// \p ModulePath.
 //
 /// This includes summaries from that module (in case any global summary based

lib/Analysis/ModuleSummaryAnalysis.cpp

 //===- ModuleSummaryAnalysis.cpp - Module summary index builder -----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This pass builds a ModuleSummaryIndex object for the module, to be written
 // to bitcode or LLVM assembly.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/ModuleSummaryAnalysis.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
case Intrinsic::type_checked_load: {
 
     break;
   }
   default:
     break;
   }
 }
 
+static bool isNonVolatileLoad(const Instruction *I) {
+  if (const auto *LI = dyn_cast<LoadInst>(I))
+    return !LI->isVolatile();
+
+  return false;
+}
+
+static void processInstructionRefs(SetVector<ValueInfo> &FuncRefs,
+                                   const Instruction *I,
+                                   SetVector<ValueInfo> &InstRefs,
+                                   DenseMap<ValueInfo, bool> &RefAccessMap) {
+  for (auto &VI : InstRefs) {
+    bool NonVolatileLoad = isNonVolatileLoad(I);
+    auto P = RefAccessMap.insert({VI, NonVolatileLoad});
+    if (!P.second && !NonVolatileLoad)
+      (*P.first).second = false;
+
+    FuncRefs.insert(VI);
+  }
+}
+
+static BitVector computeRefAccessBits(SetVector<ValueInfo> &Refs,
+                                      DenseMap<ValueInfo, bool> &RefAccessMap) {
+  BitVector Res;
+  Res.resize(Refs.size());
+  int I = 0;
+
+  for (auto &VI : Refs)
+    Res[I++] = RefAccessMap[VI];
+  return Res;
+}
+
 static void
 computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
                        const Function &F, BlockFrequencyInfo *BFI,
                        ProfileSummaryInfo *PSI, bool HasLocalsInUsedOrAsm,
                        DenseSet<GlobalValue::GUID> &CantBePromoted) {
   // Summary not currently supported for anonymous functions, they should
   // have been named.
   assert(F.hasName());
computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
   SetVector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls,
       TypeCheckedLoadConstVCalls;
   ICallPromotionAnalysis ICallAnalysis;
   SmallPtrSet<const User *, 8> Visited;
 
   // Add personality function, prefix data and prologue data to function's ref
   // list.
   findRefEdges(Index, &F, RefEdges, Visited);
+  DenseMap<ValueInfo, bool> RefAccessMap;
 
   bool HasInlineAsmMaybeReferencingInternal = false;
   for (const BasicBlock &BB : F)
     for (const Instruction &I : BB) {
       if (isa<DbgInfoIntrinsic>(I))
         continue;
       ++NumInsts;
-      findRefEdges(Index, &I, RefEdges, Visited);
+
+      SetVector<ValueInfo> InstRefEdges;
+      findRefEdges(Index, &I, InstRefEdges, Visited);
+      processInstructionRefs(RefEdges, &I, InstRefEdges, RefAccessMap);
+
       auto CS = ImmutableCallSite(&I);
       if (!CS)
         continue;
 
       const auto *CI = dyn_cast<CallInst>(&I);
       // Since we don't know exactly which local values are referenced in inline
       // assembly, conservatively mark the function as possibly referencing
       // a local value from inline assembly to ensure we don't export a
for (auto &I : F.getImportGUIDs())
     CallGraphEdges[Index.getOrInsertValueInfo(I)].updateHotness(
         ForceSummaryEdgesCold == FunctionSummary::FSHT_All
             ? CalleeInfo::HotnessType::Cold
             : CalleeInfo::HotnessType::Critical);
 
   bool NonRenamableLocal = isNonRenamableLocal(F);
   bool NotEligibleForImport =
       NonRenamableLocal || HasInlineAsmMaybeReferencingInternal ||
       // Inliner doesn't handle variadic functions.
       // FIXME: refactor this to use the same code that inliner is using.
       F.isVarArg() ||
       // Don't try to import functions with noinline attribute.
       F.getAttributes().hasFnAttribute(Attribute::NoInline);
   GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
                                     /* Live = */ false, F.isDSOLocal());
+  BitVector RefAccessBits = computeRefAccessBits(RefEdges, RefAccessMap);
   FunctionSummary::FFlags FunFlags{
       F.hasFnAttribute(Attribute::ReadNone),
       F.hasFnAttribute(Attribute::ReadOnly),
       F.hasFnAttribute(Attribute::NoRecurse),
       F.returnDoesNotAlias(),
   };
   auto FuncSummary = llvm::make_unique<FunctionSummary>(
       Flags, NumInsts, FunFlags, RefEdges.takeVector(),
       CallGraphEdges.takeVector(), TypeTests.takeVector(),
       TypeTestAssumeVCalls.takeVector(), TypeCheckedLoadVCalls.takeVector(),
       TypeTestAssumeConstVCalls.takeVector(),
       TypeCheckedLoadConstVCalls.takeVector());
   if (NonRenamableLocal)
     CantBePromoted.insert(F.getGUID());
+  FuncSummary->setRefAccessBits(std::move(RefAccessBits));
   Index.addGlobalValueSummary(F, std::move(FuncSummary));
 }
 
 static void
 computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V,
                        DenseSet<GlobalValue::GUID> &CantBePromoted) {
   SetVector<ValueInfo> RefEdges;
   SmallPtrSet<const User *, 8> Visited;
   findRefEdges(Index, &V, RefEdges, Visited);
   bool NonRenamableLocal = isNonRenamableLocal(V);
   GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal,
                                     /* Live = */ false, V.isDSOLocal());
   auto GVarSummary =
       llvm::make_unique<GlobalVarSummary>(Flags, RefEdges.takeVector());
   if (NonRenamableLocal)
     CantBePromoted.insert(V.getGUID());
   Index.addGlobalValueSummary(V, std::move(GVarSummary));
 }
 
 static void
 computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A,
                     DenseSet<GlobalValue::GUID> &CantBePromoted) {
   bool NonRenamableLocal = isNonRenamableLocal(A);
   GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal,

lib/Bitcode/Reader/BitcodeReader.cpp

static void parseTypeIdSummaryRecord(ArrayRef<uint64_t> Record,
   TypeId.TTRes.SizeM1 = Record[Slot++];
   TypeId.TTRes.BitMask = Record[Slot++];
   TypeId.TTRes.InlineBits = Record[Slot++];
 
   while (Slot < Record.size())
     parseWholeProgramDevirtResolution(Record, Strtab, Slot, TypeId);
 }
 
+static BitVector computeRefAccessBits(const std::vector<uint64_t> &RORefIDs,
+                                      size_t NumRefs) {
+  BitVector Res;
+  Res.resize(NumRefs);
+
+  for (auto RefID : RORefIDs)
+    Res.set(RefID);
+  return Res;
+}
+
 // Eagerly parse the entire summary block. This populates the GlobalValueSummary
 // objects in the index.
 Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
   if (Stream.EnterSubBlock(ID))
     return error("Invalid record");
   SmallVector<uint64_t, 64> Record;
 
   // Parse version
Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
   // each function summary records; these variables store the information
   // collected so far so that it can be used to create the summary object.
   std::vector<GlobalValue::GUID> PendingTypeTests;
   std::vector<FunctionSummary::VFuncId> PendingTypeTestAssumeVCalls,
       PendingTypeCheckedLoadVCalls;
   std::vector<FunctionSummary::ConstVCall> PendingTypeTestAssumeConstVCalls,
       PendingTypeCheckedLoadConstVCalls;
 
+  // Contains numbers of RO references.
+  std::vector<uint64_t> RORefIDs;
+
   while (true) {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
 
     switch (Entry.Kind) {
     case BitstreamEntry::SubBlock: // Handled for us already.
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
case bitc::FS_PERMODULE_PROFILE: {
              "Record size inconsistent with number of references");
       std::vector<ValueInfo> Refs = makeRefList(
           ArrayRef<uint64_t>(Record).slice(RefListStartIndex, NumRefs));
       bool HasProfile = (BitCode == bitc::FS_PERMODULE_PROFILE);
       bool HasRelBF = (BitCode == bitc::FS_PERMODULE_RELBF);
       std::vector<FunctionSummary::EdgeTy> Calls = makeCallList(
           ArrayRef<uint64_t>(Record).slice(CallGraphEdgeStartIndex),
           IsOldProfileFormat, HasProfile, HasRelBF);
+      BitVector RefAccessBits = computeRefAccessBits(RORefIDs, Refs.size());
       auto FS = llvm::make_unique<FunctionSummary>(
           Flags, InstCount, getDecodedFFlags(RawFunFlags), std::move(Refs),
           std::move(Calls), std::move(PendingTypeTests),
           std::move(PendingTypeTestAssumeVCalls),
           std::move(PendingTypeCheckedLoadVCalls),
           std::move(PendingTypeTestAssumeConstVCalls),
           std::move(PendingTypeCheckedLoadConstVCalls));
       PendingTypeTests.clear();
       PendingTypeTestAssumeVCalls.clear();
       PendingTypeCheckedLoadVCalls.clear();
       PendingTypeTestAssumeConstVCalls.clear();
       PendingTypeCheckedLoadConstVCalls.clear();
+      RORefIDs.clear();
       auto VIAndOriginalGUID = getValueInfoFromValueId(ValueID);
       FS->setModulePath(getThisModule()->first());
       FS->setOriginalName(VIAndOriginalGUID.second);
+      FS->setRefAccessBits(std::move(RefAccessBits));
       TheIndex.addGlobalValueSummary(VIAndOriginalGUID.first, std::move(FS));
       break;
     }
     // FS_ALIAS: [valueid, flags, valueid]
     // Aliases must be emitted (and parsed) after all FS_PERMODULE entries, as
     // they expect all aliasee summaries to be available.
     case bitc::FS_ALIAS: {
       unsigned ValueID = Record[0];
case bitc::FS_COMBINED_PROFILE: {
              "Record size inconsistent with number of references");
       std::vector<ValueInfo> Refs = makeRefList(
           ArrayRef<uint64_t>(Record).slice(RefListStartIndex, NumRefs));
       bool HasProfile = (BitCode == bitc::FS_COMBINED_PROFILE);
       std::vector<FunctionSummary::EdgeTy> Edges = makeCallList(
           ArrayRef<uint64_t>(Record).slice(CallGraphEdgeStartIndex),
           IsOldProfileFormat, HasProfile, false);
       ValueInfo VI = getValueInfoFromValueId(ValueID).first;
+      BitVector RefAccessBits = computeRefAccessBits(RORefIDs, Refs.size());
       auto FS = llvm::make_unique<FunctionSummary>(
           Flags, InstCount, getDecodedFFlags(RawFunFlags), std::move(Refs),
           std::move(Edges), std::move(PendingTypeTests),
           std::move(PendingTypeTestAssumeVCalls),
           std::move(PendingTypeCheckedLoadVCalls),
           std::move(PendingTypeTestAssumeConstVCalls),
           std::move(PendingTypeCheckedLoadConstVCalls));
       PendingTypeTests.clear();
       PendingTypeTestAssumeVCalls.clear();
       PendingTypeCheckedLoadVCalls.clear();
       PendingTypeTestAssumeConstVCalls.clear();
       PendingTypeCheckedLoadConstVCalls.clear();
+      RORefIDs.clear();
       LastSeenSummary = FS.get();
       LastSeenGUID = VI.getGUID();
       FS->setModulePath(ModuleIdMap[ModuleId]);
+      FS->setRefAccessBits(std::move(RefAccessBits));
       TheIndex.addGlobalValueSummary(VI, std::move(FS));
       break;
     }
     // FS_COMBINED_ALIAS: [valueid, modid, flags, valueid]
     // Aliases must be emitted (and parsed) after all FS_COMBINED entries, as
     // they expect all aliasee summaries to be available.
     case bitc::FS_COMBINED_ALIAS: {
       unsigned ValueID = Record[0];
case bitc::FS_COMBINED_ORIGINAL_NAME: {
         return error("Name attachment that does not follow a combined record");
       LastSeenSummary->setOriginalName(OriginalName);
       TheIndex.addOriginalName(LastSeenGUID, OriginalName);
       // Reset the LastSeenSummary
       LastSeenSummary = nullptr;
       LastSeenGUID = 0;
       break;
     }
+    case bitc::FS_CONST_REF_LIST:
+      assert(RORefIDs.empty());
+      RORefIDs.insert(RORefIDs.end(), Record.begin(), Record.end());
+      break;
+
     case bitc::FS_TYPE_TESTS:
       assert(PendingTypeTests.empty());
       PendingTypeTests.insert(PendingTypeTests.end(), Record.begin(),
                               Record.end());
       break;
 
     case bitc::FS_TYPE_TEST_ASSUME_VCALLS:
       assert(PendingTypeTestAssumeVCalls.empty());

lib/Bitcode/Writer/BitcodeWriter.cpp

static void writeTypeIdSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
   NameVals.push_back(Summary.TTRes.BitMask);
   NameVals.push_back(Summary.TTRes.InlineBits);
 
   for (auto &W : Summary.WPDRes)
     writeWholeProgramDevirtResolution(NameVals, StrtabBuilder, W.first,
                                       W.second);
 }
 
+static void
+writeFunctionRefAccess(BitstreamWriter &Stream, GlobalValueSummary *Summary,
+                       llvm::function_ref<bool(const ValueInfo &)> Filter) {
+  SmallVector<uint64_t, 64> Record;
+  auto *FS = cast<FunctionSummary>(Summary);
+  const BitVector &BV = FS->getRefAccessBits();
+
+  ArrayRef<ValueInfo> Refs = FS->refs();
+  assert(BV.empty() || BV.size() == Refs.size());
+
+  // Unabbreviated records are 6 bits VBR encoded. To be more space efficient
+  // we emit numbers of read-only refs.
+  for (size_t I = 0; I < BV.size(); ++I)
+    if (BV[I] && Filter(Refs[I]))
+      Record.push_back(I);
+
+  if (!Record.empty())
+    Stream.EmitRecord(bitc::FS_CONST_REF_LIST, Record);
+}
+
 // Helper to emit a single function summary record.
 void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
     SmallVector<uint64_t, 64> &NameVals, GlobalValueSummary *Summary,
     unsigned ValueID, unsigned FSCallsAbbrev, unsigned FSCallsProfileAbbrev,
     const Function &F) {
   NameVals.push_back(ValueID);
 
   FunctionSummary *FS = cast<FunctionSummary>(Summary);
   std::set<GlobalValue::GUID> ReferencedTypeIds;
   writeFunctionTypeMetadataRecords(Stream, FS, ReferencedTypeIds);
+  writeFunctionRefAccess(Stream, FS, [](const ValueInfo &Ref) { return true; });
 
   NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
   NameVals.push_back(FS->instCount());
   NameVals.push_back(getEncodedFFlags(FS->fflags()));
   NameVals.push_back(FS->refs().size());
 
   for (auto &RI : FS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
if (auto *VS = dyn_cast<GlobalVarSummary>(S)) {
                         FSModRefsAbbrev);
       NameVals.clear();
       MaybeEmitOriginalName(*S);
       return;
     }
 
     auto *FS = cast<FunctionSummary>(S);
     writeFunctionTypeMetadataRecords(Stream, FS, ReferencedTypeIds);
+    writeFunctionRefAccess(Stream, FS, [this](const ValueInfo &Ref) {
+      return !!getValueId(Ref.getGUID());
+    });
 
     NameVals.push_back(*ValueId);
     NameVals.push_back(Index.getModuleId(FS->modulePath()));
     NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
     NameVals.push_back(FS->instCount());
     NameVals.push_back(getEncodedFFlags(FS->fflags()));
     // Fill in below
     NameVals.push_back(0);
for (auto &EI : FS->calls()) {
         GUID = Index.getGUIDFromOriginalID(GUID);
         if (GUID == 0)
           continue;
         CallValueId = getValueId(GUID);
         if (!CallValueId)
           continue;
         // The mapping from OriginalId to GUID may return a GUID
         // that corresponds to a static variable. Filter it out here.
-        // This can happen when 
+        // This can happen when
         // 1) There is a call to a library function which does not have
         // a CallValidId;
         // 2) There is a static variable with the  OriginalGUID identical
         // to the GUID of the library function in 1);
         // When this happens, the logic for SamplePGO kicks in and
         // the static variable in 2) will be found, which needs to be
         // filtered out.
         auto *GVSum = Index.getGlobalValueSummary(GUID, false);

lib/IR/ModuleSummaryIndex.cpp

return getSummaryList().size() &&
                       [](const std::unique_ptr<GlobalValueSummary> &Summary) {
                         return Summary->isDSOLocal();
                       });
 }
 
 // Collect for the given module the list of function it defines
 // (GUID -> Summary).
 void ModuleSummaryIndex::collectDefinedFunctionsForModule(
     StringRef ModulePath, GVSummaryMapTy &GVSummaryMap) const {
   for (auto &GlobalList : *this) {
     auto GUID = GlobalList.first;
     for (auto &GlobSummary : GlobalList.second.SummaryList) {
       auto *Summary = dyn_cast_or_null<FunctionSummary>(GlobSummary.get());
       if (!Summary)
         // Ignore global variable, focus on functions
         continue;
       // Ignore summaries from other modules.
bool ModuleSummaryIndex::isGUIDLive(GlobalValue::GUID GUID) const {
   if (SummaryList.empty())
     return true;
   for (auto &I : SummaryList)
     if (isGlobalValueLive(I.get()))
       return true;
   return false;
 }
 
+void ModuleSummaryIndex::propagateConstantsForFunction(FunctionSummary *FS) {
+  int RefNo = 0;
+  if (!FS->isLive())
+    return;
+
+  const BitVector &BV = FS->getRefAccessBits();
+  for (auto &VI : FS->refs()) {
+    for (auto &Summary : VI.getSummaryList())
+      if (!BV[RefNo])
+        Summary->setConstant(false);
+    ++RefNo;
+  }
+}
+
+void ModuleSummaryIndex::collectMutableGVSummaries(
+    GlobalValueSummary *RootGVS, DenseSet<GlobalValueSummary *> &MutableGVSet) {
+  if (!RootGVS->isLive() || RootGVS->isConstant())
+    return;
+
+  SmallVector<GlobalValueSummary *, 128> Worklist;
+  if (MutableGVSet.insert(RootGVS).second)
+    Worklist.push_back(RootGVS);
+
+  while (!Worklist.empty()) {
+    auto *GVS = Worklist.pop_back_val();
+    for (auto &VI : GVS->refs())
+      for (auto &Summary : VI.getSummaryList())
+        if (Summary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
+            MutableGVSet.insert(Summary.get()).second)
+          Worklist.push_back(Summary.get());
+  }
+}
+
+void ModuleSummaryIndex::propagateConstants(
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) {
+  auto ForEachSummary =
+      [this](
+          llvm::function_ref<void(GlobalValue::GUID, GlobalValueSummary *)> F) {
+        for (auto &VI : *this)
+          for (auto &Summary : VI.second.SummaryList)
+            F(VI.first, Summary.get());
+      };
+  // Step 1: mark all summaries corresponding to global variables constant.
+  // We don't want to mark exported symbols constant. We also don't currently
+  // support import of global variables which have refs.
+  ForEachSummary([&](GlobalValue::GUID Id, GlobalValueSummary *S) {
+    S->setConstant(S->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
+                   S->refs().empty() && !GUIDPreservedSymbols.count(Id));
+  });
+
+  // Step 2: for each function summary check its refs. If ref is not constant
+  // then referenced summary list is not constant either.
+  ForEachSummary([this](GlobalValue::GUID, GlobalValueSummary *S) {
+    if (auto *FS = dyn_cast<FunctionSummary>(S))
+      propagateConstantsForFunction(FS);
+  });
+
+  // Step 3: for each global variable check if it is still constant. If it isn't
+  // then all its refs are not constant either.
+  DenseSet<GlobalValueSummary *> MutableGVSet;
+  ForEachSummary([&](GlobalValue::GUID, GlobalValueSummary *S) {
+    if (S->getSummaryKind() == GlobalValueSummary::GlobalVarKind)
+      collectMutableGVSummaries(S, MutableGVSet);
+  });
+
+  for (auto *Summary : MutableGVSet)
+    Summary->setConstant(false);
+
+  // Step 4: aliases inherit constant attribute from aliasee.
+  ForEachSummary([this](GlobalValue::GUID, GlobalValueSummary *S) {
+    if (auto *AS = dyn_cast<AliasSummary>(S))
+      AS->setConstant(AS->getAliasee().isConstant());
+  });
+}
+
 // TODO: write a graphviz dumper for SCCs (see ModuleSummaryIndex::exportToDot)
 // then delete this function and update its tests
 LLVM_DUMP_METHOD
 void ModuleSummaryIndex::dumpSCCs(raw_ostream &O) {
   for (scc_iterator<ModuleSummaryIndex *> I =
            scc_begin<ModuleSummaryIndex *>(this);
        !I.isAtEnd(); ++I) {
     O << "SCC (" << utostr(I->size()) << " node" << (I->size() == 1 ? "" : "s")
       << ") {\n";
     for (const ValueInfo V : *I) {
       FunctionSummary *F = nullptr;
       if (V.getSummaryList().size())
         F = cast<FunctionSummary>(V.getSummaryList().front().get());
       O << " " << (F == nullptr ? "External" : "") << " " << utostr(V.getGUID())
         << (I.hasLoop() ? " (has loop)" : "") << "\n";
     }
     O << "}\n";
   }
 }
 
 namespace {
 struct Attributes {
   void add(const Twine &Name, const Twine &Value,
            const Twine &Comment = Twine());
+  void addComment(const Twine &Comment);
   std::string getAsString() const;
 
   std::vector<std::string> Attrs;
   std::string Comments;
 };
 
 struct Edge {
   uint64_t SrcMod;
   int Hotness;
   GlobalValue::GUID Src;
   GlobalValue::GUID Dst;
 };
 }
 
 void Attributes::add(const Twine &Name, const Twine &Value,
                      const Twine &Comment) {
   std::string A = Name.str();
   A += "=\"";
   A += Value.str();
   A += "\"";
   Attrs.push_back(A);
+  addComment(Comment);
+}
+
+void Attributes::addComment(const Twine &Comment) {
   if (!Comment.isTriviallyEmpty()) {
     if (Comments.empty())
       Comments = " // ";
     else
       Comments += ", ";
     Comments += Comment.str();
   }
 }

 // or variable defined in native object or library.
 static void defineExternalNode(raw_ostream &OS, const char *Pfx,
                                const ValueInfo &VI) {
   auto StrId = std::to_string(VI.getGUID());
   OS << "  " << StrId << " [label=\"" << getNodeVisualName(VI)
      << "\"]; // defined externally\n";
 }
 
+static bool isReadOnlyRef(GlobalValueSummary *GVS, const ValueInfo &Ref) {
+  auto *FS = dyn_cast<FunctionSummary>(GVS);
+  if (!FS)
+    return false;
+
+  const BitVector &BV = FS->getRefAccessBits();
+  if (BV.empty())
+    return false;
+
+  ArrayRef<ValueInfo> FuncRefs = FS->refs();
+  return BV[&Ref - &FuncRefs[0]];
+}
+
 void ModuleSummaryIndex::exportToDot(raw_ostream& OS) const {
   std::vector<Edge> CrossModuleEdges;
   DenseMap<GlobalValue::GUID, std::vector<uint64_t>> NodeMap;
   StringMap<GVSummaryMapTy> ModuleToDefinedGVS;
   collectDefinedGVSummariesPerModule(ModuleToDefinedGVS);
 
   // Get node identifier in form MXXX_<GUID>. The MXXX prefix is required,
   // because we may have multiple linkonce functions summaries.
   auto NodeId = [](uint64_t ModId, GlobalValue::GUID Id) {
     return ModId == (uint64_t)-1 ? std::to_string(Id)
                                  : std::string("M") + std::to_string(ModId) +
                                        "_" + std::to_string(Id);
   };
 
   auto DrawEdge = [&](const char *Pfx, int SrcMod, GlobalValue::GUID SrcId,
                       int DstMod, GlobalValue::GUID DstId, int TypeOrHotness) {
-    // 0 corresponds to alias edge, 1 to ref edge, 2 to call with unknown
-    // hotness, ...
-    TypeOrHotness += 2;
+    // 0 - alias
+    // 1 - reference
+    // 2 - constant reference
+    // Other value: (hotness - 3).
+    TypeOrHotness += 3;
     static const char *EdgeAttrs[] = {
         " [style=dotted]; // alias",
         " [style=dashed]; // ref",
+        " [style=dashed,color=forestgreen]; // const-ref",
         " // call (hotness : Unknown)",
         " [color=blue]; // call (hotness : Cold)",
         " // call (hotness : None)",
         " [color=brown]; // call (hotness : Hot)",
         " [style=bold,color=red]; // call (hotness : Critical)"};
 
     assert(static_cast<size_t>(TypeOrHotness) <
            sizeof(EdgeAttrs) / sizeof(EdgeAttrs[0]));
for (auto &SummaryIt : GVSMap) {
       Attributes A;
       if (isa<FunctionSummary>(SummaryIt.second)) {
         A.add("shape", "record", "function");
       } else if (isa<AliasSummary>(SummaryIt.second)) {
         A.add("style", "dotted,filled", "alias");
         A.add("shape", "box");
       } else {
         A.add("shape", "Mrecord", "variable");
+        if (Flags.Live && SummaryIt.second->isConstant())
+          A.addComment("immutable");
       }
 
       auto VI = getValueInfo(SummaryIt.first);
       A.add("label", getNodeLabel(VI, SummaryIt.second));
       if (!Flags.Live)
         A.add("fillcolor", "red", "dead");
       else if (Flags.NotEligibleToImport)
         A.add("fillcolor", "yellow", "not eligible to import");
 
       OS << "    " << NodeId(ModId, SummaryIt.first) << " " << A.getAsString()
          << "\n";
     }
     OS << "    // Edges:\n";
 
     for (auto &SummaryIt : GVSMap) {
       auto *GVS = SummaryIt.second;
       for (auto &R : GVS->refs())
-        Draw(SummaryIt.first, R.getGUID(), -1);
+        Draw(SummaryIt.first, R.getGUID(), isReadOnlyRef(GVS, R) ? -1 : -2);
 
       if (auto *AS = dyn_cast_or_null<AliasSummary>(SummaryIt.second)) {
         auto AliaseeOrigId = AS->getAliasee().getOriginalName();
         auto AliaseeId = getGUIDFromOriginalID(AliaseeOrigId);
 
-        Draw(SummaryIt.first, AliaseeId ? AliaseeId : AliaseeOrigId, -2);
+        Draw(SummaryIt.first, AliaseeId ? AliaseeId : AliaseeOrigId, -3);
         continue;
       }
 
       if (auto *FS = dyn_cast_or_null<FunctionSummary>(SummaryIt.second))
         for (auto &CGEdge : FS->calls())
           Draw(SummaryIt.first, CGEdge.first.getGUID(),
                static_cast<int>(CGEdge.second.Hotness));
     }

lib/LTO/LTO.cpp

#endif
   // sensitive to link order, so include that as well.
   for (auto &Entry : ImportList) {
     auto ModHash = Index.getModuleHash(Entry.first());
     Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
 
     AddUint64(Entry.second.size());
     for (auto &Fn : Entry.second)
       AddUint64(Fn);
   }
 
   // Include the hash for the resolved ODR.
   for (auto &Entry : ResolvedODR) {
     Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&Entry.first,
                                     sizeof(GlobalValue::GUID)));
     Hasher.update(ArrayRef<uint8_t>((const uint8_t *)&Entry.second,
                                     sizeof(GlobalValue::LinkageTypes)));
   }
thinLTOResolveWeakForLinkerGUID(I.second.SummaryList, I.first,
                                     GlobalInvolvedWithAlias, isPrevailing,
                                     recordNewLinkage);
 }
 
 static void thinLTOInternalizeAndPromoteGUID(
     GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
     function_ref<bool(StringRef, GlobalValue::GUID)> isExported) {
   for (auto &S : GVSummaryList) {
-    if (isExported(S->modulePath(), GUID)) {
+    if (!S->isConstant() && isExported(S->modulePath(), GUID)) {
       if (GlobalValue::isLocalLinkage(S->linkage()))
         S->setLinkage(GlobalValue::ExternalLinkage);
     } else if (!GlobalValue::isLocalLinkage(S->linkage()))
       S->setLinkage(GlobalValue::InternalLinkage);
   }
 }
 
 // Update the linkages in the given \p Index to mark exported values
for (GlobalValue *GV : Mod.Keep) {
     if (CombinedGV && !CombinedGV->isDeclaration())
       continue;
 
     Keep.push_back(GV);
   }
 
   return RegularLTO.Mover->move(std::move(Mod.M), Keep,
                                 [](GlobalValue &, IRMover::ValueAdder) {},
+                                [](GlobalValue *, GlobalValue *) {},
                                 /* IsPerformingImport */ false);
 }
 
 // Add a ThinLTO module to the link.
 Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
                       const SymbolResolution *&ResI,
                       const SymbolResolution *ResE) {
   if (Error Err =
Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache) {
   }
 
   auto isPrevailing = [&](GlobalValue::GUID G) {
     auto It = GUIDPrevailingResolutions.find(G);
     if (It == GUIDPrevailingResolutions.end())
       return PrevailingType::Unknown;
     return It->second;
   };
-  computeDeadSymbols(ThinLTO.CombinedIndex, GUIDPreservedSymbols, isPrevailing);
+  computeDeadSymbolsWithConstProp(ThinLTO.CombinedIndex, GUIDPreservedSymbols,
+                                  isPrevailing);
 
   // Setup output file to emit statistics.
   std::unique_ptr<ToolOutputFile> StatsFile = nullptr;
   if (!Conf.StatsFile.empty()) {
     EnableStatistics(false);
     std::error_code EC;
     StatsFile =
         llvm::make_unique<ToolOutputFile>(Conf.StatsFile, EC, sys::fs::F_None);

lib/LTO/ThinLTOCodeGenerator.cpp

static void computeDeadSymbolsInIndex(
     ModuleSummaryIndex &Index,
     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) {
   // We have no symbols resolution available. And can't do any better now in the
   // case where the prevailing symbol is in a native object. It can be refined
   // with linker information in the future.
   auto isPrevailing = [&](GlobalValue::GUID G) {
     return PrevailingType::Unknown;
   };
-  computeDeadSymbols(Index, GUIDPreservedSymbols, isPrevailing);
+  computeDeadSymbolsWithConstProp(Index, GUIDPreservedSymbols, isPrevailing);
 }
 
 /**
  * Perform promotion and renaming of exported internal functions.
  * Index is updated to reflect linkage changes from weak resolution.
  */
 void ThinLTOCodeGenerator::promote(Module &TheModule,
                                    ModuleSummaryIndex &Index) {
   auto ModuleCount = Index.modulePaths().size();

lib/Linker/IRMover.cpp

 /// from SrcM to DstM.
 class IRLinker {
   Module &DstM;
   std::unique_ptr<Module> SrcM;
 
   /// See IRMover::move().
   std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor;
 
+  std::function<void(GlobalValue *, GlobalValue *)> MaterializerHook;
+
   TypeMapTy TypeMap;
   GlobalValueMaterializer GValMaterializer;
   LocalValueMaterializer LValMaterializer;
 
   /// A metadata map that's shared between IRLinker instances.
   MDMapT &SharedMDs;
 
   /// Mapping of values from what they used to be in Src, to what they are now
class IRLinker {
   void prepareCompileUnitsForImport();
   void linkNamedMDNodes();
 
 public:
   IRLinker(Module &DstM, MDMapT &SharedMDs,
            IRMover::IdentifiedStructTypeSet &Set, std::unique_ptr<Module> SrcM,
            ArrayRef<GlobalValue *> ValuesToLink,
            std::function<void(GlobalValue &, IRMover::ValueAdder)> AddLazyFor,
+           std::function<void(GlobalValue *, GlobalValue *)> MaterializerHook,
            bool IsPerformingImport)
       : DstM(DstM), SrcM(std::move(SrcM)), AddLazyFor(std::move(AddLazyFor)),
-        TypeMap(Set), GValMaterializer(*this), LValMaterializer(*this),
-        SharedMDs(SharedMDs), IsPerformingImport(IsPerformingImport),
+        MaterializerHook(std::move(MaterializerHook)), TypeMap(Set),
+        GValMaterializer(*this), LValMaterializer(*this), SharedMDs(SharedMDs),
+        IsPerformingImport(IsPerformingImport),
         Mapper(ValueMap, RF_MoveDistinctMDs | RF_IgnoreMissingLocals, &TypeMap,
                &GValMaterializer),
         AliasMCID(Mapper.registerAlternateMappingContext(AliasValueMap,
                                                          &LValMaterializer)) {
     ValueMap.getMDMap() = std::move(SharedMDs);
     for (GlobalValue *GV : ValuesToLink)
       maybeAdd(GV);
     if (IsPerformingImport)
Value *IRLinker::materialize(Value *V, bool ForAlias) {
   }
   if (!*NewProto)
     return nullptr;
 
   GlobalValue *New = dyn_cast<GlobalValue>(*NewProto);
   if (!New)
     return *NewProto;
 
+  if (MaterializerHook)
+    MaterializerHook(New, SGV);
+
   // If we already created the body, just return.
   if (auto *F = dyn_cast<Function>(New)) {
     if (!F->isDeclaration())
       return New;
   } else if (auto *V = dyn_cast<GlobalVariable>(New)) {
     if (V->hasInitializer() || V->hasAppendingLinkage())
       return New;
   } else {
for (unsigned I = 0, E = SrcCompileUnits->getNumOperands(); I != E; ++I) {
     // Enums, macros, and retained types don't need to be listed on the
     // imported DICompileUnit. This means they will only be imported
     // if reached from the mapped IR. Do this by setting their value map
     // entries to nullptr, which will automatically prevent their importing
     // when reached from the DICompileUnit during metadata mapping.
     ValueMap.MD()[CU->getRawEnumTypes()].reset(nullptr);
     ValueMap.MD()[CU->getRawMacros()].reset(nullptr);
     ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
-    // We import global variables only temporarily in order for instcombine
-    // and globalopt to perform constant folding and static constructor
-    // evaluation. After that elim-avail-extern will covert imported globals
-    // back to declarations, so we don't need debug info for them.
-    ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
 
     // Imported entities only need to be mapped in if they have local
     // scope, as those might correspond to an imported entity inside a
     // function being imported (any locally scoped imported entities that
     // don't end up referenced by an imported function will not be emitted
     // into the object). Imported entities not in a local scope
     // (e.g. on the namespace) only need to be emitted by the originating
     // module. Create a list of the locally scoped imported entities, and
IRMover::IRMover(Module &M) : Composite(M) {
   for (auto *MD : StructTypes.getVisitedMetadata()) {
     SharedMDs[MD].reset(const_cast<MDNode *>(MD));
   }
 }
 
 Error IRMover::move(
     std::unique_ptr<Module> Src, ArrayRef<GlobalValue *> ValuesToLink,
     std::function<void(GlobalValue &, ValueAdder Add)> AddLazyFor,
+    std::function<void(GlobalValue *, GlobalValue *)> MaterializerHook,
     bool IsPerformingImport) {
   IRLinker TheIRLinker(Composite, SharedMDs, IdentifiedStructTypes,
                        std::move(Src), ValuesToLink, std::move(AddLazyFor),
-                       IsPerformingImport);
+                       std::move(MaterializerHook), IsPerformingImport);
   Error E = TheIRLinker.run();
   Composite.dropTriviallyDeadConstantArrays();
   return E;
 }

lib/Linker/LinkModules.cpp

bool ModuleLinker::run() {
 
   // FIXME: Propagate Errors through to the caller instead of emitting
   // diagnostics.
   bool HasErrors = false;
   if (Error E = Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(),
                            [this](GlobalValue &GV, IRMover::ValueAdder Add) {
                              addLazyFor(GV, Add);
                            },
+                           [](GlobalValue *, GlobalValue *) {},
                            /* IsPerformingImport */ false)) {
     handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) {
       DstM.getContext().diagnose(LinkDiagnosticInfo(DS_Error, EIB.message()));
       HasErrors = true;
     });
   }
   if (HasErrors)
     return true;

lib/Transforms/IPO/FunctionImport.cpp

for (auto &VI : Summary.refs()) {
     }
 
     LLVM_DEBUG(dbgs() << " ref -> " << VI << "\n");
 
     for (auto &RefSummary : VI.getSummaryList())
       if (RefSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
           // Don't try to import regular LTO summaries added to dummy module.
           !RefSummary->modulePath().empty() &&
           !GlobalValue::isInterposableLinkage(RefSummary->linkage()) &&
           RefSummary->refs().empty()) {
         ImportList[RefSummary->modulePath()].insert(VI.getGUID());
         if (ExportLists)
           (*ExportLists)[RefSummary->modulePath()].insert(VI.getGUID());
         break;
       }
   }
 }
void llvm::computeDeadSymbols(
 
   unsigned DeadSymbols = Index.size() - LiveSymbols;
   LLVM_DEBUG(dbgs() << LiveSymbols << " symbols Live, and " << DeadSymbols
                     << " symbols Dead \n");
   NumDeadSymbols += DeadSymbols;
   NumLiveSymbols += LiveSymbols;
 }
 
+// Compute dead symbols and propagate constants in combined index.
+void llvm::computeDeadSymbolsWithConstProp(
+    ModuleSummaryIndex &Index,
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
+    function_ref<PrevailingType(GlobalValue::GUID)> isPrevailing) {
+  computeDeadSymbols(Index, GUIDPreservedSymbols, isPrevailing);
+  Index.propagateConstants(GUIDPreservedSymbols);
+}
+
 /// Compute the set of summaries needed for a ThinLTO backend compilation of
 /// \p ModulePath.
 void llvm::gatherImportedSummariesForModule(
     StringRef ModulePath,
     const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
     const FunctionImporter::ImportMapTy &ImportList,
     std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
   // Include all summaries from the importing module.
   ModuleToSummariesForIndex[ModulePath] =
       ModuleToDefinedGVSummaries.lookup(ModulePath);
   // Include summaries for imports.
static Function *replaceAliasWithAliasee(Module *SrcModule, GlobalAlias *GA) {
   // Clone should use the original alias's linkage and name, and we ensure
   // all uses of alias instead use the new clone (casted if necessary).
   NewFn->setLinkage(GA->getLinkage());
   GA->replaceAllUsesWith(ConstantExpr::getBitCast(NewFn, GA->getType()));
   NewFn->takeName(GA);
   return NewFn;
 }
 
+// Checks if global value is immutable according to combined summary.
+static bool isImmutableGlobal(const ModuleSummaryIndex &Index,
+                              const GlobalValue *GV) {
+  auto *GVS = Index.getGlobalValueSummary(GV->getGUID(), false);
+  return GVS->isConstant();
+}
+
 // Automatically import functions in Module \p DestModule based on the summaries
 // index.
 Expected<bool> FunctionImporter::importFunctions(
     Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
   LLVM_DEBUG(dbgs() << "Starting import for Module "
                     << DestModule.getModuleIdentifier() << "\n");
   unsigned ImportedCount = 0, ImportedGVCount = 0;
 
   IRMover Mover(DestModule);
+  DenseSet<GlobalValue *> ImmutableGVsMoved;
   // Do the actual import of functions now, one Module at a time
   std::set<StringRef> ModuleNameOrderedList;
   for (auto &FunctionsToImportPerModule : ImportList) {
     ModuleNameOrderedList.insert(FunctionsToImportPerModule.first());
   }
   for (auto &Name : ModuleNameOrderedList) {
+    DenseSet<GlobalValue *> ImmutableGVs;
     // Get the module for the import
     const auto &FunctionsToImportPerModule = ImportList.find(Name);
     assert(FunctionsToImportPerModule != ImportList.end());
     Expected<std::unique_ptr<Module>> SrcModuleOrErr = ModuleLoader(Name);
     if (!SrcModuleOrErr)
       return SrcModuleOrErr.takeError();
     std::unique_ptr<Module> SrcModule = std::move(*SrcModuleOrErr);
     assert(&DestModule.getContext() == &SrcModule->getContext() &&
for (GlobalVariable &GV : SrcModule->globals()) {
       auto Import = ImportGUIDs.count(GUID);
       LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing global "
                         << GUID << " " << GV.getName() << " from "
                         << SrcModule->getSourceFileName() << "\n");
       if (Import) {
         if (Error Err = GV.materialize())
           return std::move(Err);
         ImportedGVCount += GlobalsToImport.insert(&GV);
+
+        // Immutable globals are not promoted, so we need to make a local copy.
+        // We are doing this by internalizing imported definitions later.
+        if (isImmutableGlobal(Index, &GV))
+          ImmutableGVs.insert(&GV);
       }
     }
     for (GlobalAlias &GA : SrcModule->aliases()) {
       if (!GA.hasName())
         continue;
       auto GUID = GA.getGUID();
       auto Import = ImportGUIDs.count(GUID);
       LLVM_DEBUG(dbgs() << (Import ? "Is" : "Not") << " importing alias "
for (auto &Name : ModuleNameOrderedList) {
     if (PrintImports) {
       for (const auto *GV : GlobalsToImport)
         dbgs() << DestModule.getSourceFileName() << ": Import " << GV->getName()
                << " from " << SrcModule->getSourceFileName() << "\n";
     }
 
     if (Mover.move(std::move(SrcModule), GlobalsToImport.getArrayRef(),
                    [](GlobalValue &, IRMover::ValueAdder) {},
+                   [&](GlobalValue *DGV, GlobalValue *SGV) {
+                     if (ImmutableGVs.count(SGV))
+                       ImmutableGVsMoved.insert(DGV);
+                   },
                    /*IsPerformingImport=*/true))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += GlobalsToImport.size();
     NumImportedModules++;
   }
 
+  // Internalize immutable globals.
+  for (auto *MGV : ImmutableGVsMoved)
+    MGV->setLinkage(GlobalValue::InternalLinkage);
+
   NumImportedFunctions += (ImportedCount - ImportedGVCount);
   NumImportedGlobalVars += ImportedGVCount;
 
   LLVM_DEBUG(dbgs() << "Imported " << ImportedCount - ImportedGVCount
                     << " functions for Module "
                     << DestModule.getModuleIdentifier() << "\n");
   LLVM_DEBUG(dbgs() << "Imported " << ImportedGVCount
                     << " global variables for Module "

test/Bitcode/thinlto-function-summary-callgraph-relbf.ll

 ; Test to check the callgraph in summary
 ; RUN: opt -write-relbf-to-summary -module-summary %s -o %t.o
 ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
 ; RUN: llvm-dis -o - %t.o | FileCheck %s --check-prefix=DIS
 ; Round trip it through llvm-as
 ; RUN: llvm-dis -o - %t.o | llvm-as -write-relbf-to-summary -o - | llvm-dis -o - | FileCheck %s --check-prefix=DIS
 
 ; CHECK: <SOURCE_FILENAME
 ; CHECK-NEXT: <GLOBALVAR
 ; CHECK-NEXT: <FUNCTION
 ; "func"
 ; CHECK-NEXT: <FUNCTION op0=17 op1=4
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered.
+; CHECK-NEXT:    <CONST_REF_LIST op0=0/>
 ; CHECK-NEXT:    <PERMODULE_RELBF {{.*}} op4=1 {{.*}} op7=256
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'undefinedglobmainfunc{{.*}}'
 
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

test/Bitcode/thinlto-function-summary-callgraph.ll

 
 ; CHECK: <SOURCE_FILENAME
 ; CHECK-NEXT: <GLOBALVAR
 ; CHECK-NEXT: <FUNCTION
 ; "func"
 ; CHECK-NEXT: <FUNCTION op0=17 op1=4
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
-; See if the call to func is registered.
+; See if the call to func is registered
+; CHECK-NEXT:    <CONST_REF_LIST op0=0/>
 ; CHECK-NEXT:    <PERMODULE {{.*}} op4=1
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'undefinedglobmainfunc{{.*}}'
 
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION

test/ThinLTO/X86/Inputs/globals-import.ll

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
-@baz = internal constant i32 10, align 4
+@baz = internal global i32 10, align 4
 
 define linkonce_odr i32 @foo() {
+  ; Prevent thinlto from internalizing read-only variable @baz
+  store i32 20, i32* @baz, align 4
   %1 = load i32, i32* @baz, align 4
   ret i32 %1
 }

test/ThinLTO/X86/Inputs/index-const-prop.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+@gBar = local_unnamed_addr global i32 2, align 4, !dbg !0
+@gFoo = internal unnamed_addr global i32 1, align 4, !dbg !6
+
+; Function Attrs: norecurse nounwind readonly
+define i32 @foo() local_unnamed_addr #0 !dbg !14 {
+  %1 = load i32, i32* @gFoo, align 4, !dbg !17
+  ret i32 %1, !dbg !18
+}
+
+; Function Attrs: norecurse nounwind readonly
+define i32 @bar() local_unnamed_addr #0 !dbg !19 {
+  %1 = load i32, i32* @gBar, align 4, !dbg !20
+  ret i32 %1, !dbg !21
+}
+
+define void @baz() local_unnamed_addr !dbg !22 {
+  %1 = tail call i32 @rand(), !dbg !25
+  store i32 %1, i32* @gFoo, align 4, !dbg !26
+  %2 = tail call i32 @rand(), !dbg !27
+  store i32 %2, i32* @gBar, align 4, !dbg !28
+  ret void, !dbg !29
+}
+
+declare i32 @rand() local_unnamed_addr
+
+attributes #0 = { norecurse nounwind readonly }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!9, !10, !11, !12}
+!llvm.ident = !{!13}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "gBar", scope: !2, file: !3, line: 4, type: !8, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 7.0.0 (trunk 332246)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "foo.c", directory: "/data/work/lto/roref/test")
+!4 = !{}
+!5 = !{!0, !6}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = distinct !DIGlobalVariable(name: "gFoo", scope: !2, file: !3, line: 3, type: !8, isLocal: true, isDefinition: true)
+!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!9 = !{i32 2, !"Dwarf Version", i32 4}
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"wchar_size", i32 4}
+!12 = !{i32 7, !"PIC Level", i32 2}
+!13 = !{!"clang version 7.0.0 (trunk 332246)"}
+!14 = distinct !DISubprogram(name: "foo", scope: !3, file: !3, line: 6, type: !15, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: true, unit: !2, retainedNodes: !4)
+!15 = !DISubroutineType(types: !16)
+!16 = !{!8}
+!17 = !DILocation(line: 7, column: 10, scope: !14)
+!18 = !DILocation(line: 7, column: 3, scope: !14)
+!19 = distinct !DISubprogram(name: "bar", scope: !3, file: !3, line: 10, type: !15, isLocal: false, isDefinition: true, scopeLine: 10, isOptimized: true, unit: !2, retainedNodes: !4)
+!20 = !DILocation(line: 11, column: 10, scope: !19)
+!21 = !DILocation(line: 11, column: 3, scope: !19)
+!22 = distinct !DISubprogram(name: "baz", scope: !3, file: !3, line: 14, type: !23, isLocal: false, isDefinition: true, scopeLine: 14, isOptimized: true, unit: !2, retainedNodes: !4)
+!23 = !DISubroutineType(types: !24)
+!24 = !{null}
+!25 = !DILocation(line: 15, column: 10, scope: !22)
+!26 = !DILocation(line: 15, column: 8, scope: !22)
+!27 = !DILocation(line: 16, column: 10, scope: !22)
+!28 = !DILocation(line: 16, column: 8, scope: !22)
+!29 = !DILocation(line: 17, column: 1, scope: !22)

test/ThinLTO/X86/dot-dumper.ll

 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER0 %s
 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER1 %s
 
 ; STRUCTURE:        digraph Summary {
 ; STRUCTURE-DAG:      subgraph cluster_0
 ; STRUCTURE-DAG:      subgraph cluster_1
 ; STRUCTURE:          // Cross-module edges:
 ; STRUCTURE-DAG:      M0_{{[0-9]+}} -> M1_{{[0-9]+}} // call
-; STRUCTURE-DAG:      M0_{{[0-9]+}} -> M1_{{[0-9]+}} [{{.*}}]; // ref
+; STRUCTURE-DAG:      M0_{{[0-9]+}} -> M1_{{[0-9]+}} [{{.*}}]; // const-ref
 ; STRUCTURE-NEXT:   }
 
 ; CLUSTER0:         // Module: {{.*}}1.bc
 ; CLUSTER0-NEXT:    subgraph cluster_0 {
 ; CLUSTER0-DAG:       M0_[[MAIN_ALIAS:[0-9]+]] [{{.*}}main_alias{{.*}}]; // alias, dead
 ; CLUSTER0-DAG:       M0_[[MAIN:[0-9]+]] [{{.*}}main|extern{{.*}}]; // function
 ; CLUSTER0-NEXT:      // Edges:
 ; CLUSTER0-NEXT:      M0_[[MAIN_ALIAS]] -> M0_[[MAIN]] [{{.*}}]; // alias
 ; CLUSTER0-NEXT:    }
 
 ; CLUSTER1:         // Module: {{.*}}2.bc
 ; CLUSTER1-NEXT:    subgraph cluster_1 {
-; CLUSTER1-DAG:       M1_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable
+; CLUSTER1-DAG:       M1_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable, immutable
 ; CLUSTER1-DAG:       M1_[[FOO:[0-9]+]] [{{.*}}foo|extern{{.*}}]; // function, not eligible to import
-; CLUSTER1-DAG:       M1_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable
+; CLUSTER1-DAG:       M1_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable, immutable
 ; CLUSTER1-DAG:       M1_[[BAR:[0-9]+]] [{{.*}}bar|extern{{.*}}]; // function, dead
 ; CLUSTER1-NEXT:      // Edges:
-; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[B]] [{{.*}}]; // ref
-; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[A]] [{{.*}}]; // ref
+; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[B]] [{{.*}}]; // const-ref
+; CLUSTER1-DAG:       M1_[[FOO]] -> M1_[[A]] [{{.*}}]; // const-ref
 ; CLUSTER1-DAG:     }
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @A = external local_unnamed_addr global i32, align 4
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() local_unnamed_addr {
   %1 = tail call i32 (...) @foo()
   %2 = load i32, i32* @A, align 4
   %3 = add nsw i32 %2, %1
   ret i32 %3
 }
 @main_alias = weak_odr alias i32 (), i32 ()* @main
 declare i32 @foo(...) local_unnamed_addr

test/ThinLTO/X86/export.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/export.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t1.bc %t2.bc
 
 ; Ensure statics are promoted/renamed correctly from this file.
 ; RUN: llvm-lto -thinlto-action=promote %t1.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s
-; CHECK-DAG: @staticvar.llvm.0 = hidden global
+; CHECK-DAG: @staticvar = internal global
 ; CHECK-DAG: define hidden void @staticfunc.llvm.0
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.11.0"
 
 @staticvar = internal global i32 1, align 4
 
 define void @callstaticfunc() #0 {

test/ThinLTO/X86/funcimport.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/funcimport.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.bc %t.bc %t2.bc
 
 ; RUN: llvm-lto -thinlto-index-stats %t3.bc | FileCheck %s -check-prefix=STATS
 ; STATS: Index {{.*}} contains 24 nodes (13 functions, 3 alias, 8 globals) and 19 edges (8 refs and 11 calls)
 
 ; Ensure statics are promoted/renamed correctly from this file (all but
 ; constant variable need promotion).
 ; RUN: llvm-lto -thinlto-action=promote %t.bc -thinlto-index=%t3.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=EXPORTSTATIC
-; EXPORTSTATIC-DAG: @staticvar.llvm.0 = hidden global
+; EXPORTSTATIC-DAG: @staticvar = internal global
 ; Eventually @staticconstvar can be exported as a copy and not promoted
-; EXPORTSTATIC-DAG: @staticconstvar.llvm.0 = hidden unnamed_addr constant
-; EXPORTSTATIC-DAG: @P.llvm.0 = hidden global void ()* null
+; EXPORTSTATIC-DAG: @staticconstvar = internal unnamed_addr constant
+; EXPORTSTATIC-DAG: @P = internal global void ()* null
 ; EXPORTSTATIC-DAG: define hidden i32 @staticfunc.llvm.0
 ; EXPORTSTATIC-DAG: define hidden void @staticfunc2.llvm.0
 
 ; Ensure that weak alias to an imported function is correctly turned into
 ; a declaration.
 ; Also ensures that alias to a linkonce function is turned into a declaration
 ; and that the associated linkonce function is not in the output, as it is
 ; lazily linked and never referenced/materialized.

test/ThinLTO/X86/globals-import-const-fold.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/globals-import-cf-baz.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc -o %t3.index.bc
 
 ; RUN: llvm-lto -thinlto-action=import %t1.bc %t2.bc -thinlto-index=%t3.index.bc
 ; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %s
 ; RUN: llvm-lto -thinlto-action=optimize %t1.bc.thinlto.imported.bc -o %t1.bc.thinlto.opt.bc
 ; RUN: llvm-dis %t1.bc.thinlto.opt.bc -o - | FileCheck --check-prefix=OPTIMIZE %s
 
-; IMPORT: @baz = available_externally local_unnamed_addr constant i32 10
+; IMPORT: @baz = internal local_unnamed_addr constant i32 10
 
 ; OPTIMIZE:       define i32 @main()
 ; OPTIMIZE-NEXT:    ret i32 10
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 @baz = external local_unnamed_addr constant i32, align 4
 
 define i32 @main() local_unnamed_addr {
   %1 = load i32, i32* @baz, align 4
   ret i32 %1
 }

test/ThinLTO/X86/globals-import.ll

 ; Test to ensure that we import a single copy of a global variable. This is
 ; important when we link in an object file twice, which is normally works when
 ; all symbols have either weak or internal linkage. If we import an internal
 ; global variable twice it will get promoted in each module, and given the same
 ; name as the IR hash will be identical, resulting in multiple defs when linking.
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/globals-import.ll -o %t2.bc
 ; RUN: opt -module-summary %p/Inputs/globals-import.ll -o %t2b.bc
 ; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc %t2b.bc -o %t3.index.bc
 
-; RUN: llvm-lto -thinlto-action=import %t1.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-lto -thinlto-action=import %t1.bc -exported-symbol=main -thinlto-index=%t3.index.bc
 ; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %s
-; RUN: llvm-lto -thinlto-action=promote %t2.bc -thinlto-index=%t3.index.bc
-; RUN: llvm-lto -thinlto-action=promote %t2b.bc -thinlto-index=%t3.index.bc
+; RUN: llvm-lto -thinlto-action=promote %t2.bc -exported-symbol=main -thinlto-index=%t3.index.bc
+; RUN: llvm-lto -thinlto-action=promote %t2b.bc -exported-symbol=main -thinlto-index=%t3.index.bc
 ; RUN: llvm-dis %t2.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE1 %s
 ; RUN: llvm-dis %t2b.bc.thinlto.promoted.bc -o - | FileCheck --check-prefix=PROMOTE2 %s
 
-; IMPORT: @baz.llvm.0 = available_externally hidden constant i32 10, align 4
+; IMPORT: @baz.llvm.0 = available_externally hidden global i32 10, align 4
 
-; PROMOTE1: @baz.llvm.0 = hidden constant i32 10, align 4
+; PROMOTE1: @baz.llvm.0 = hidden global i32 10, align 4
 ; PROMOTE1: define weak_odr i32 @foo() {
 
 ; Second copy of IR object should not have any symbols imported/promoted.
-; PROMOTE2: @baz = internal constant i32 10, align 4
+; PROMOTE2: @baz = internal global i32 10, align 4
 ; PROMOTE2: define available_externally i32 @foo() {
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-linux-gnu"
 
 declare i32 @foo()
 
 define i32 @main() local_unnamed_addr {
   %1 = call i32 @foo()
   ret i32 %1
 }

test/ThinLTO/X86/index-const-prop.ll

+; Check constant propagation in thinlto combined summary. This allows us to do 2 things:
+;  1. Internalize global definition which is not used externally if all accesses to it are read-only
+;  2. Make a local copy of internal definition if all accesses to it are readonly. This allows constant
+;     folding it during optimziation phase.
+
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop.ll -o %t2.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t3.index.bc %t1.bc %t2.bc
+; RUN: llvm-lto -thinlto-action=import  %t1.bc -thinlto-index=%t3.index.bc -o %t1.imported.bc
+; RUN: llvm-dis %t1.imported.bc -o - | FileCheck %s --check-prefix=IMPORT
+; RUN: llvm-lto -thinlto-action=optimize %t1.imported.bc -o - | llvm-dis - -o - | FileCheck %s --check-prefix=OPTIMIZE
+
+; Check that we don't internalize gBar when it is exported
+; RUN: llvm-lto -thinlto-action=import -exported-symbol main -exported-symbol gBar  %t1.bc -thinlto-index=%t3.index.bc -o %t1.imported2.bc
+; RUN: llvm-dis %t1.imported2.bc -o - | FileCheck %s --check-prefix=IMPORT2
+
+; IMPORT: @gFoo.llvm.0 = internal unnamed_addr global i32 1, align 4, !dbg !0
+; IMPORT-NEXT: @gBar = internal local_unnamed_addr global i32 2, align 4, !dbg !5
+; IMPORT: !DICompileUnit({{.*}}, globals: !{{[0-9]+}})
+
+; OPTIMIZE:        define i32 @main
+; OPTIMIZE-NEXT:     ret i32 3
+
+; IMPORT2: @gBar = available_externally local_unnamed_addr global i32 2, align 4, !dbg !5
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+define i32 @main() local_unnamed_addr {
+  %call = tail call i32 bitcast (i32 (...)* @foo to i32 ()*)()
+  %call1 = tail call i32 bitcast (i32 (...)* @bar to i32 ()*)()
+  %add = add nsw i32 %call1, %call
+  ret i32 %add
+}
+
+declare i32 @foo(...) local_unnamed_addr
+
+declare i32 @bar(...) local_unnamed_addr

test/ThinLTO/X86/index-const-prop2.ll

+; Check constant propagation in thinlto combined summary. This allows us to do 2 things:
+;  1. Internalize global definition which is not used externally if all accesses to it are read-only
+;  2. Make a local copy of internal definition if all accesses to it are readonly. This allows constant
+;     folding it during optimziation phase.
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop.ll -o %t2.bc
+; RUN: llvm-lto2 run %t1.bc %t2.bc -save-temps \
+; RUN:  -r=%t2.bc,foo,pl \
+; RUN:  -r=%t2.bc,bar,pl \
+; RUN:  -r=%t2.bc,baz,pl \
+; RUN:  -r=%t2.bc,rand, \
+; RUN:  -r=%t2.bc,gBar,pl \
+; RUN:  -r=%t1.bc,main,plx \
+; RUN:  -r=%t1.bc,foo, \
+; RUN:  -r=%t1.bc,bar, \
+; RUN:  -o %t3
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT
+; RUN: llvm-dis %t3.1.5.precodegen.bc -o - | FileCheck %s --check-prefix=CODEGEN
+
+; Now check that we won't internalize global (gBar) if it's externally referenced
+; RUN: llvm-lto2 run %t1.bc %t2.bc -save-temps \
+; RUN:  -r=%t2.bc,foo,pl \
+; RUN:  -r=%t2.bc,bar,pl \
+; RUN:  -r=%t2.bc,baz,pl \
+; RUN:  -r=%t2.bc,rand, \
+; RUN:  -r=%t2.bc,gBar,plx \
+; RUN:  -r=%t1.bc,main,plx \
+; RUN:  -r=%t1.bc,foo, \
+; RUN:  -r=%t1.bc,bar, \
+; RUN:  -o %t3
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT2
+
+; IMPORT:       @gFoo.llvm.0 = internal unnamed_addr global i32 1, align 4
+; IMPORT-NEXT:  @gBar = internal local_unnamed_addr global i32 2, align 4
+; IMPORT:       !DICompileUnit({{.*}}, globals: !{{[0-9]+}})
+
+; CODEGEN:        i32 @main()
+; CODEGEN-NEXT:     ret i32 3
+
+; IMPORT2: @gBar = available_externally dso_local local_unnamed_addr global i32 2, align 4
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+define i32 @main() local_unnamed_addr {
+  %call = tail call i32 bitcast (i32 (...)* @foo to i32 ()*)()
+  %call1 = tail call i32 bitcast (i32 (...)* @bar to i32 ()*)()
+  %add = add nsw i32 %call1, %call
+  ret i32 %add
+}
+
+declare i32 @foo(...) local_unnamed_addr
+
+declare i32 @bar(...) local_unnamed_addr

tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

default:
       STRINGIFY_CODE(FS, TYPE_TEST_ASSUME_VCALLS)
       STRINGIFY_CODE(FS, TYPE_CHECKED_LOAD_VCALLS)
       STRINGIFY_CODE(FS, TYPE_TEST_ASSUME_CONST_VCALL)
       STRINGIFY_CODE(FS, TYPE_CHECKED_LOAD_CONST_VCALL)
       STRINGIFY_CODE(FS, VALUE_GUID)
       STRINGIFY_CODE(FS, CFI_FUNCTION_DEFS)
       STRINGIFY_CODE(FS, CFI_FUNCTION_DECLS)
       STRINGIFY_CODE(FS, TYPE_ID)
+      STRINGIFY_CODE(FS, CONST_REF_LIST)
     }
   case bitc::METADATA_ATTACHMENT_ID:
     switch(CodeID) {
     default:return nullptr;
       STRINGIFY_CODE(METADATA, ATTACHMENT)
     }
   case bitc::METADATA_BLOCK_ID:
     switch(CodeID) {
static bool ParseBlock(BitstreamCursor &Stream, BitstreamBlockInfo &BlockInfo,
   while (1) {
     if (Stream.AtEndOfStream())
       return ReportError("Premature end of bitstream");
 
     uint64_t RecordStartBit = Stream.GetCurrentBitNo();
 
     BitstreamEntry Entry =
       Stream.advance(BitstreamCursor::AF_DontAutoprocessAbbrevs);
-    
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
       return ReportError("malformed bitcode file");
     case BitstreamEntry::EndBlock: {
       uint64_t BlockBitEnd = Stream.GetCurrentBitNo();
       BlockStats.NumBits += BlockBitEnd-BlockBitStart;
       if (DumpRecords) {
         outs() << Indent << "</";
         if (BlockName)
           outs() << BlockName << ">\n";
         else
           outs() << "UnknownBlock" << BlockID << ">\n";
       }
       return false;
     }
-        
     case BitstreamEntry::SubBlock: {
       uint64_t SubBlockBitStart = Stream.GetCurrentBitNo();
       if (ParseBlock(Stream, BlockInfo, Entry.ID, IndentLevel + 1,
                      CurStreamType))
         return true;
       ++BlockStats.NumSubBlocks;
       uint64_t SubBlockBitEnd = Stream.GetCurrentBitNo();
-      
       // Don't include subblock sizes in the size of this block.
       BlockBitStart += SubBlockBitEnd-SubBlockBitStart;
       continue;
     }
     case BitstreamEntry::Record:
       // The interesting case.
       break;
     }
 
     if (Entry.ID == bitc::DEFINE_ABBREV) {
       Stream.ReadAbbrevRecord();
       ++BlockStats.NumAbbrevs;
       continue;
     }
-    
     Record.clear();
 
     ++BlockStats.NumRecords;
 
     StringRef Blob;
     uint64_t CurrentRecordPos = Stream.GetCurrentBitNo();
     unsigned Code = Stream.readRecord(Entry.ID, Record, &Blob);
 
if (DumpRecords) {
             if (!isprint(static_cast<unsigned char>(Blob[i]))) {
               BlobIsPrintable = false;
               break;
             }
 
           if (BlobIsPrintable)
             outs() << "'" << Blob << "'";
           else
-            outs() << "unprintable, " << Blob.size() << " bytes.";          
+            outs() << "unprintable, " << Blob.size() << " bytes.";
         }
       }
 
       outs() << "\n";
     }
 
     // Make sure that we can skip the current record.
     Stream.JumpToBit(CurrentRecordPos);