[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

283 ↗

(On Diff #169852)

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.

530 ↗

(On Diff #169852)

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

lib/IR/ModuleSummaryIndex.cpp

148 ↗

(On Diff #169852)

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

283 ↗

(On Diff #169852)

Makes sense, I'll give it a try

530 ↗

(On Diff #169852)

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

148 ↗

(On Diff #169852)

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

371 ↗

(On Diff #170611)

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

530 ↗

(On Diff #169852)

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

421 ↗

(On Diff #170611)

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 ↗

(On Diff #170611)

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

99 ↗

(On Diff #170611)

Just invoke S->getBaseObject() for simplicity.

106 ↗

(On Diff #170611)

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.

120 ↗

(On Diff #170611)

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?

128 ↗

(On Diff #170611)

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

132 ↗

(On Diff #170611)

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

143 ↗

(On Diff #170611)

Suggest adding the explanation of why.

lib/LTO/LTO.cpp

153 ↗

(On Diff #170611)

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

828 ↗

(On Diff #170611)

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

530 ↗

(On Diff #169852)

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

371 ↗

(On Diff #170611)

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.

421 ↗

(On Diff #170611)

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 ↗

(On Diff #170611)

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 ++;

106 ↗

(On Diff #170611)

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

153 ↗

(On Diff #170611)

Yep, I missed second call of AddUsedThings

lib/Transforms/IPO/FunctionImport.cpp

828 ↗

(On Diff #170611)

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

828 ↗

(On Diff #170611)

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

828 ↗

(On Diff #170611)

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

530 ↗

(On Diff #169852)

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

650 ↗

(On Diff #170611)

document constant parameter.

lib/Transforms/IPO/FunctionImport.cpp

828 ↗

(On Diff #170611)

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

372 ↗

(On Diff #171940)

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

lib/IR/ModuleSummaryIndex.cpp

106 ↗

(On Diff #171940)

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

149 ↗

(On Diff #171940)

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

106 ↗

(On Diff #171940)

In ValueInfo constructor we have:

RefAndFlags.setInt(HaveGVs);

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

149 ↗

(On Diff #171940)

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

106 ↗

(On Diff #171940)

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.

149 ↗

(On Diff #171940)

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

144 ↗

(On Diff #172912)

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?

145 ↗

(On Diff #172912)

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

149 ↗

(On Diff #172912)

typo: s/referneces/references/

155 ↗

(On Diff #172912)

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

298 ↗

(On Diff #172912)

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

144 ↗

(On Diff #172912)

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

145 ↗

(On Diff #172912)

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

155 ↗

(On Diff #172912)

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

144 ↗

(On Diff #172912)

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.

145 ↗

(On Diff #172912)

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.

155 ↗

(On Diff #172912)

Sure =)

[evgeny777]

lib/IR/ModuleSummaryIndex.cpp

144 ↗

(On Diff #172912)

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

145 ↗

(On Diff #172912)

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

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

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

225

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

llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h

 /// of the map is unknown, resulting in inefficiencies due to repeated
 /// insertions and resizing.
 using GlobalValueSummaryMapTy =
     std::map<GlobalValue::GUID, GlobalValueSummaryInfo>;
 
 /// Struct that holds a reference to a particular GUID in a global value
 /// summary.
 struct ValueInfo {
-  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 1, bool>
-      RefAndFlag;
+  PointerIntPair<const GlobalValueSummaryMapTy::value_type *, 2, int>
+      RefAndFlags;
 
   ValueInfo() = default;
   ValueInfo(bool HaveGVs, const GlobalValueSummaryMapTy::value_type *R) {
-    RefAndFlag.setPointer(R);
-    RefAndFlag.setInt(HaveGVs);
+    RefAndFlags.setPointer(R);
+    RefAndFlags.setInt(HaveGVs);
   }
 
   operator bool() const { return getRef(); }
 
   GlobalValue::GUID getGUID() const { return getRef()->first; }
   const GlobalValue *getValue() const {
     assert(haveGVs());
     return getRef()->second.U.GV;
   }
 
   ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
     return getRef()->second.SummaryList;
   }
 
   StringRef name() const {
     return haveGVs() ? getRef()->second.U.GV->getName()
                      : getRef()->second.U.Name;
   }
 
-  bool haveGVs() const { return RefAndFlag.getInt(); }
+  bool haveGVs() const { return RefAndFlags.getInt() & 0x1; }
+  bool isReadOnly() const { return RefAndFlags.getInt() & 0x2; }
+  void setReadOnly() { RefAndFlags.setInt(RefAndFlags.getInt() | 0x2); }
 
   const GlobalValueSummaryMapTy::value_type *getRef() const {
-    return RefAndFlag.getPointer();
+    return RefAndFlags.getPointer();
   }
 
   bool isDSOLocal() const;
 };
 
 inline raw_ostream &operator<<(raw_ostream &OS, const ValueInfo &VI) {
   OS << VI.getGUID();
   if (!VI.name().empty())
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)});
   }
+  // Gets the number of immutable refs in RefEdgeList
+  unsigned immutableRefCount() const;
 
   /// Check if this is a function summary.
   static bool classof(const GlobalValueSummary *GVS) {
     return GVS->getSummaryKind() == FunctionKind;
   }
 
   /// Get function summary flags.
   FFlags fflags() const { return FunFlags; }
template <> struct DenseMapInfo<FunctionSummary::ConstVCall> {
   static unsigned getHashValue(FunctionSummary::ConstVCall I) {
     return I.VFunc.GUID;
   }
 };
 
 /// Global variable summary information to aid decisions and
 /// implementation of importing.
 ///
-/// Currently this doesn't add anything to the base \p GlobalValueSummary,
-/// but is a placeholder as additional info may be added to the summary
-/// for variables.
+/// Global variable summary has extra flag, telling if it is
+/// modified during the program run or not. This affects ThinLTO
+/// internalization
 class GlobalVarSummary : public GlobalValueSummary {
-
 public:
-  GlobalVarSummary(GVFlags Flags, std::vector<ValueInfo> Refs)
-      : GlobalValueSummary(GlobalVarKind, Flags, std::move(Refs)) {}
+  struct GVarFlags {
+    GVarFlags(bool ReadOnly = false) : ReadOnly(ReadOnly) {}
+
+    unsigned ReadOnly : 1;
+  } VarFlags;
+
+  GlobalVarSummary(GVFlags Flags, GVarFlags VarFlags,
+                   std::vector<ValueInfo> Refs)
+      : GlobalValueSummary(GlobalVarKind, Flags, std::move(Refs)),
+        VarFlags(VarFlags) {}
 
   /// Check if this is a global variable summary.
   static bool classof(const GlobalValueSummary *GVS) {
     return GVS->getSummaryKind() == GlobalVarKind;
   }
+
+  GVarFlags varflags() const { return VarFlags; }
+  void setReadOnly(bool RO) { VarFlags.ReadOnly = RO; }
+  bool isReadOnly() const { return VarFlags.ReadOnly; }
 };
 
 struct TypeTestResolution {
   /// Specifies which kind of type check we should emit for this byte array.
   /// See http://clang.llvm.org/docs/ControlFlowIntegrityDesign.html for full
   /// details on each kind of check; the enumerators are described with
   /// reference to that document.
   enum Kind {
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;
static NodeRef getEntryNode(ModuleSummaryIndex *I) {
     GlobalValueSummaryInfo G(I->haveGVs());
     G.SummaryList.push_back(std::move(Root));
     static auto P =
         GlobalValueSummaryMapTy::value_type(GlobalValue::GUID(0), std::move(G));
     return ValueInfo(I->haveGVs(), &P);
   }
 };
 
+static inline bool canImportGlobalVar(GlobalValueSummary *S) {
+  assert(isa<GlobalVarSummary>(S->getBaseObject()));
+
+  // We don't import GV with references, because it can result
+  // in promotion of local variables in the source module.
+  return !GlobalValue::isInterposableLinkage(S->linkage()) &&
+         !S->notEligibleToImport() && S->refs().empty();
+}
 } // end namespace llvm
 
 #endif // LLVM_IR_MODULESUMMARYINDEX_H

llvm/trunk/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,
+    bool ImportEnabled);
+
 /// 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

llvm/trunk/include/llvm/Transforms/Utils/FunctionImportUtils.h

static bool doImportAsDefinition(const GlobalValue *SGV,
                                    SetVector<GlobalValue *> *GlobalsToImport);
 };
 
 /// Perform in-place global value handling on the given Module for
 /// exported local functions renamed and promoted for ThinLTO.
 bool renameModuleForThinLTO(
     Module &M, const ModuleSummaryIndex &Index,
     SetVector<GlobalValue *> *GlobalsToImport = nullptr);
-
 } // End llvm namespace
 
 #endif

llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp

case Intrinsic::type_checked_load: {
 
     break;
   }
   default:
     break;
   }
 }
 
-static void computeFunctionSummary(
-    ModuleSummaryIndex &Index, const Module &M, const Function &F,
-    BlockFrequencyInfo *BFI, ProfileSummaryInfo *PSI, DominatorTree &DT,
-    bool HasLocalsInUsedOrAsm, DenseSet<GlobalValue::GUID> &CantBePromoted) {
+static bool isNonVolatileLoad(const Instruction *I) {
+  if (const auto *LI = dyn_cast<LoadInst>(I))
+    return !LI->isVolatile();
+
+  return false;
+}
+
+static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
+                                   const Function &F, BlockFrequencyInfo *BFI,
+                                   ProfileSummaryInfo *PSI, DominatorTree &DT,
+                                   bool HasLocalsInUsedOrAsm,
+                                   DenseSet<GlobalValue::GUID> &CantBePromoted,
+                                   bool IsThinLTO) {
   // Summary not currently supported for anonymous functions, they should
   // have been named.
   assert(F.hasName());
 
   unsigned NumInsts = 0;
   // Map from callee ValueId to profile count. Used to accumulate profile
   // counts for all static calls to a given callee.
   MapVector<ValueInfo, CalleeInfo> CallGraphEdges;
   SetVector<ValueInfo> RefEdges;
   SetVector<GlobalValue::GUID> TypeTests;
   SetVector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
       TypeCheckedLoadVCalls;
   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);
+  std::vector<const Instruction *> NonVolatileLoads;
 
   bool HasInlineAsmMaybeReferencingInternal = false;
   for (const BasicBlock &BB : F)
     for (const Instruction &I : BB) {
       if (isa<DbgInfoIntrinsic>(I))
         continue;
       ++NumInsts;
+      if (isNonVolatileLoad(&I)) {
+        // Postpone processing of non-volatile load instructions
+        // See comments below
+        Visited.insert(&I);
+        NonVolatileLoads.push_back(&I);
+        continue;
+      }
       findRefEdges(Index, &I, RefEdges, Visited);
       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
for (const Instruction &I : BB) {
             ICallAnalysis.getPromotionCandidatesForInstruction(
                 &I, NumVals, TotalCount, NumCandidates);
         for (auto &Candidate : CandidateProfileData)
           CallGraphEdges[Index.getOrInsertValueInfo(Candidate.Value)]
               .updateHotness(getHotness(Candidate.Count, PSI));
       }
     }
 
+  // By now we processed all instructions in a function, except
+  // non-volatile loads. All new refs we add in a loop below
+  // are obviously constant. All constant refs are grouped in the
+  // end of RefEdges vector, so we can use a single integer value
+  // to identify them.
+  unsigned RefCnt = RefEdges.size();
+  for (const Instruction *I : NonVolatileLoads) {
+    Visited.erase(I);
+    findRefEdges(Index, I, RefEdges, Visited);
+  }
+  std::vector<ValueInfo> Refs = RefEdges.takeVector();
+  // Regular LTO module doesn't participate in ThinLTO import,
+  // so no reference from it can be readonly, since this would
+  // require importing variable as local copy
+  if (IsThinLTO)
+    for (; RefCnt < Refs.size(); ++RefCnt)
+      Refs[RefCnt].setReadOnly();
+
   // Explicit add hot edges to enforce importing for designated GUIDs for
   // sample PGO, to enable the same inlines as the profiled optimized binary.
   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;
   GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
                                     /* Live = */ false, F.isDSOLocal());
   FunctionSummary::FFlags FunFlags{
       F.hasFnAttribute(Attribute::ReadNone),
       F.hasFnAttribute(Attribute::ReadOnly),
       F.hasFnAttribute(Attribute::NoRecurse), F.returnDoesNotAlias(),
       // 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)};
   auto FuncSummary = llvm::make_unique<FunctionSummary>(
-      Flags, NumInsts, FunFlags, RefEdges.takeVector(),
-      CallGraphEdges.takeVector(), TypeTests.takeVector(),
-      TypeTestAssumeVCalls.takeVector(), TypeCheckedLoadVCalls.takeVector(),
+      Flags, NumInsts, FunFlags, std::move(Refs), CallGraphEdges.takeVector(),
+      TypeTests.takeVector(), TypeTestAssumeVCalls.takeVector(),
+      TypeCheckedLoadVCalls.takeVector(),
       TypeTestAssumeConstVCalls.takeVector(),
       TypeCheckedLoadConstVCalls.takeVector());
   if (NonRenamableLocal)
     CantBePromoted.insert(F.getGUID());
   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;
   bool HasBlockAddress = 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());
+
+  // Don't mark variables we won't be able to internalize as read-only.
+  GlobalVarSummary::GVarFlags VarFlags(
+      !V.hasComdat() && !V.hasAppendingLinkage() && !V.isInterposable() &&
+      !V.hasAvailableExternallyLinkage() && !V.hasDLLExportStorageClass());
+  auto GVarSummary = llvm::make_unique<GlobalVarSummary>(Flags, VarFlags,
+                                                         RefEdges.takeVector());
   if (NonRenamableLocal)
     CantBePromoted.insert(V.getGUID());
   if (HasBlockAddress)
     GVarSummary->setNotEligibleToImport();
   Index.addGlobalValueSummary(V, std::move(GVarSummary));
 }
 
 static void
ModuleSymbolTable::CollectAsmSymbols(
                     ArrayRef<GlobalValue::GUID>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::ConstVCall>{},
                     ArrayRef<FunctionSummary::ConstVCall>{});
             Index.addGlobalValueSummary(*GV, std::move(Summary));
           } else {
             std::unique_ptr<GlobalVarSummary> Summary =
-                llvm::make_unique<GlobalVarSummary>(GVFlags,
-                                                    ArrayRef<ValueInfo>{});
+                llvm::make_unique<GlobalVarSummary>(
+                    GVFlags, GlobalVarSummary::GVarFlags(),
             Index.addGlobalValueSummary(*GV, std::move(Summary));
           }
         });
   }
 
+  bool IsThinLTO = true;
+  if (auto *MD =
+          mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("ThinLTO")))
+    IsThinLTO = MD->getZExtValue();
+
   // Compute summaries for all functions defined in module, and save in the
   // index.
   for (auto &F : M) {
     if (F.isDeclaration())
       continue;
 
     DominatorTree DT(const_cast<Function &>(F));
     BlockFrequencyInfo *BFI = nullptr;
     std::unique_ptr<BlockFrequencyInfo> BFIPtr;
     if (GetBFICallback)
       BFI = GetBFICallback(F);
     else if (F.hasProfileData()) {
       LoopInfo LI{DT};
       BranchProbabilityInfo BPI{F, LI};
       BFIPtr = llvm::make_unique<BlockFrequencyInfo>(F, BPI, LI);
       BFI = BFIPtr.get();
     }
 
     computeFunctionSummary(Index, M, F, BFI, PSI, DT,
                            !LocalsUsed.empty() || HasLocalInlineAsmSymbol,
-                           CantBePromoted);
+                           CantBePromoted, IsThinLTO);
   }
 
   // Compute summaries for all variables defined in module, and save in the
   // index.
   for (const GlobalVariable &G : M.globals()) {
     if (G.isDeclaration())
       continue;
     computeVariableSummary(Index, G, CantBePromoted);
ModuleSummaryIndex llvm::buildModuleSummaryIndex(
   // to flag them as live in the index to ensure index-based dead value
   // analysis treats them as live roots of the analysis.
   setLiveRoot(Index, "llvm.used");
   setLiveRoot(Index, "llvm.compiler.used");
   setLiveRoot(Index, "llvm.global_ctors");
   setLiveRoot(Index, "llvm.global_dtors");
   setLiveRoot(Index, "llvm.global.annotations");
 
-  bool IsThinLTO = true;
-  if (auto *MD =
-          mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("ThinLTO")))
-    IsThinLTO = MD->getZExtValue();
-
   for (auto &GlobalList : Index) {
     // Ignore entries for references that are undefined in the current module.
     if (GlobalList.second.SummaryList.empty())
       continue;
 
     assert(GlobalList.second.SummaryList.size() == 1 &&
            "Expected module's index to have one summary per GUID");
     auto &Summary = GlobalList.second.SummaryList[0];

llvm/trunk/lib/AsmParser/LLParser.cpp

bool LLParser::ParseVariableSummary(std::string Name, GlobalValue::GUID GUID,
   if (EatIfPresent(lltok::comma)) {
     if (ParseOptionalRefs(Refs))
       return true;
   }
 
   if (ParseToken(lltok::rparen, "expected ')' here"))
     return true;
 
-  auto GS = llvm::make_unique<GlobalVarSummary>(GVFlags, std::move(Refs));
+  auto GS = llvm::make_unique<GlobalVarSummary>(
+      GVFlags, GlobalVarSummary::GVarFlags(), std::move(Refs));
 
   GS->setModulePath(ModulePath);
 
   AddGlobalValueToIndex(Name, GUID, (GlobalValue::LinkageTypes)GVFlags.Linkage,
                         ID, std::move(GS));
 
   return false;
 }

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags,
   // to work correctly on earlier versions, we must conservatively treat all
   // values as live.
   bool Live = (RawFlags & 0x2) || Version < 3;
   bool Local = (RawFlags & 0x4);
 
   return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live, Local);
 }
 
+// Decode the flags for GlobalVariable in the summary
+static GlobalVarSummary::GVarFlags getDecodedGVarFlags(uint64_t RawFlags) {
+  return GlobalVarSummary::GVarFlags((RawFlags & 0x1) ? true : false);
+}
+
 static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) {
   switch (Val) {
   default: // Map unknown visibilities to default.
   case 0: return GlobalValue::DefaultVisibility;
   case 1: return GlobalValue::HiddenVisibility;
   case 2: return GlobalValue::ProtectedVisibility;
   }
 }
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 void setImmutableRefs(std::vector<ValueInfo> &Refs, unsigned Count) {
+  // Read-only refs are in the end of the refs list.
+  for (unsigned RefNo = Refs.size() - Count; RefNo < Refs.size(); ++RefNo)
+    Refs[RefNo].setReadOnly();
+}
+
 // 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
   {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
     if (Entry.Kind != BitstreamEntry::Record)
       return error("Invalid Summary Block: record for version expected");
     if (Stream.readRecord(Entry.ID, Record) != bitc::FS_VERSION)
       return error("Invalid Summary Block: version expected");
   }
   const uint64_t Version = Record[0];
   const bool IsOldProfileFormat = Version == 1;
-  if (Version < 1 || Version > 4)
+  if (Version < 1 || Version > 5)
     return error("Invalid summary version " + Twine(Version) +
-                 ", 1, 2, 3 or 4 expected");
+                 ", 1, 2, 3, 4 or 5 expected");
   Record.clear();
 
   // Keep around the last seen summary to be used when we see an optional
   // "OriginalName" attachement.
   GlobalValueSummary *LastSeenSummary = nullptr;
   GlobalValue::GUID LastSeenGUID = 0;
 
   // We can expect to see any number of type ID information records before
while (true) {
     case bitc::FS_PERMODULE:
     case bitc::FS_PERMODULE_RELBF:
     case bitc::FS_PERMODULE_PROFILE: {
       unsigned ValueID = Record[0];
       uint64_t RawFlags = Record[1];
       unsigned InstCount = Record[2];
       uint64_t RawFunFlags = 0;
       unsigned NumRefs = Record[3];
+      unsigned NumImmutableRefs = 0;
       int RefListStartIndex = 4;
       if (Version >= 4) {
         RawFunFlags = Record[3];
         NumRefs = Record[4];
         RefListStartIndex = 5;
+        if (Version >= 5) {
+          NumImmutableRefs = Record[5];
+          RefListStartIndex = 6;
+        }
       }
 
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
       // The module path string ref set in the summary must be owned by the
       // index's module string table. Since we don't have a module path
       // string table section in the per-module index, we create a single
       // module path string table entry with an empty (0) ID to take
       // ownership.
       int CallGraphEdgeStartIndex = RefListStartIndex + NumRefs;
       assert(Record.size() >= RefListStartIndex + NumRefs &&
              "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);
+      setImmutableRefs(Refs, NumImmutableRefs);
       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();
case bitc::FS_ALIAS: {
       AS->setAliasee(AliaseeInModule);
       AS->setAliaseeGUID(AliaseeGUID);
 
       auto GUID = getValueInfoFromValueId(ValueID);
       AS->setOriginalName(GUID.second);
       TheIndex.addGlobalValueSummary(GUID.first, std::move(AS));
       break;
     }
-    // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, n x valueid]
+    // FS_PERMODULE_GLOBALVAR_INIT_REFS: [valueid, flags, varflags, n x valueid]
     case bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t RawFlags = Record[1];
+      unsigned RefArrayStart = 2;
+      GlobalVarSummary::GVarFlags GVF;
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
+      if (Version >= 5) {
+        GVF = getDecodedGVarFlags(Record[2]);
+        RefArrayStart = 3;
+      }
       std::vector<ValueInfo> Refs =
-          makeRefList(ArrayRef<uint64_t>(Record).slice(2));
-      auto FS = llvm::make_unique<GlobalVarSummary>(Flags, std::move(Refs));
+          makeRefList(ArrayRef<uint64_t>(Record).slice(RefArrayStart));
+      auto FS =
+          llvm::make_unique<GlobalVarSummary>(Flags, GVF, std::move(Refs));
       FS->setModulePath(getThisModule()->first());
       auto GUID = getValueInfoFromValueId(ValueID);
       FS->setOriginalName(GUID.second);
       TheIndex.addGlobalValueSummary(GUID.first, std::move(FS));
       break;
     }
     // FS_COMBINED: [valueid, modid, flags, instcount, fflags, numrefs,
     //               numrefs x valueid, n x (valueid)]
     // FS_COMBINED_PROFILE: [valueid, modid, flags, instcount, fflags, numrefs,
     //                       numrefs x valueid, n x (valueid, hotness)]
     case bitc::FS_COMBINED:
     case bitc::FS_COMBINED_PROFILE: {
       unsigned ValueID = Record[0];
       uint64_t ModuleId = Record[1];
       uint64_t RawFlags = Record[2];
       unsigned InstCount = Record[3];
       uint64_t RawFunFlags = 0;
       unsigned NumRefs = Record[4];
+      unsigned NumImmutableRefs = 0;
       int RefListStartIndex = 5;
 
       if (Version >= 4) {
         RawFunFlags = Record[4];
         NumRefs = Record[5];
         RefListStartIndex = 6;
+        if (Version >= 5) {
+          NumImmutableRefs = Record[6];
+          RefListStartIndex = 7;
+        }
       }
 
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
       int CallGraphEdgeStartIndex = RefListStartIndex + NumRefs;
       assert(Record.size() >= RefListStartIndex + NumRefs &&
              "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;
+      setImmutableRefs(Refs, NumImmutableRefs);
       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();
case bitc::FS_COMBINED_ALIAS: {
       TheIndex.addGlobalValueSummary(VI, std::move(AS));
       break;
     }
     // FS_COMBINED_GLOBALVAR_INIT_REFS: [valueid, modid, flags, n x valueid]
     case bitc::FS_COMBINED_GLOBALVAR_INIT_REFS: {
       unsigned ValueID = Record[0];
       uint64_t ModuleId = Record[1];
       uint64_t RawFlags = Record[2];
+      unsigned RefArrayStart = 3;
+      GlobalVarSummary::GVarFlags GVF;
       auto Flags = getDecodedGVSummaryFlags(RawFlags, Version);
+      if (Version >= 5) {
+        GVF = getDecodedGVarFlags(Record[3]);
+        RefArrayStart = 4;
+      }
       std::vector<ValueInfo> Refs =
-          makeRefList(ArrayRef<uint64_t>(Record).slice(3));
-      auto FS = llvm::make_unique<GlobalVarSummary>(Flags, std::move(Refs));
+          makeRefList(ArrayRef<uint64_t>(Record).slice(RefArrayStart));
+      auto FS =
+          llvm::make_unique<GlobalVarSummary>(Flags, GVF, std::move(Refs));
       LastSeenSummary = FS.get();
       FS->setModulePath(ModuleIdMap[ModuleId]);
       ValueInfo VI = getValueInfoFromValueId(ValueID).first;
       LastSeenGUID = VI.getGUID();
       TheIndex.addGlobalValueSummary(VI, std::move(FS));
       break;
     }
     // FS_COMBINED_ORIGINAL_NAME: [original_name]

llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) {
   // Linkage don't need to be remapped at that time for the summary. Any future
   // change to the getEncodedLinkage() function will need to be taken into
   // account here as well.
   RawFlags = (RawFlags << 4) | Flags.Linkage; // 4 bits
 
   return RawFlags;
 }
 
+static uint64_t getEncodedGVarFlags(GlobalVarSummary::GVarFlags Flags) {
+  uint64_t RawFlags = Flags.ReadOnly;
+  return RawFlags;
+}
+
 static unsigned getEncodedVisibility(const GlobalValue &GV) {
   switch (GV.getVisibility()) {
   case GlobalValue::DefaultVisibility:   return 0;
   case GlobalValue::HiddenVisibility:    return 1;
   case GlobalValue::ProtectedVisibility: return 2;
   }
   llvm_unreachable("Invalid visibility");
 }
void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
 
   FunctionSummary *FS = cast<FunctionSummary>(Summary);
   writeFunctionTypeMetadataRecords(Stream, FS);
 
   NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
   NameVals.push_back(FS->instCount());
   NameVals.push_back(getEncodedFFlags(FS->fflags()));
   NameVals.push_back(FS->refs().size());
+  NameVals.push_back(FS->immutableRefCount());
 
   for (auto &RI : FS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
 
   bool HasProfileData =
       F.hasProfileData() || ForceSummaryEdgesCold != FunctionSummary::FSHT_None;
   for (auto &ECI : FS->calls()) {
     NameVals.push_back(getValueId(ECI.first));
if (!VI || VI.getSummaryList().empty()) {
     // have a summary if the def was in module level asm).
     assert(V.isDeclaration());
     return;
   }
   auto *Summary = VI.getSummaryList()[0].get();
   NameVals.push_back(VE.getValueID(&V));
   GlobalVarSummary *VS = cast<GlobalVarSummary>(Summary);
   NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
+  NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
 
   unsigned SizeBeforeRefs = NameVals.size();
   for (auto &RI : VS->refs())
     NameVals.push_back(VE.getValueID(RI.getValue()));
   // Sort the refs for determinism output, the vector returned by FS->refs() has
   // been initialized from a DenseSet.
   llvm::sort(NameVals.begin() + SizeBeforeRefs, NameVals.end());
 
   Stream.EmitRecord(bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS, NameVals,
                     FSModRefsAbbrev);
   NameVals.clear();
 }
 
 // Current version for the summary.
 // This is bumped whenever we introduce changes in the way some record are
 // interpreted, like flags for instance.
-static const uint64_t INDEX_VERSION = 4;
+static const uint64_t INDEX_VERSION = 5;
 
 /// Emit the per-module summary section alongside the rest of
 /// the module's bitcode.
 void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   // By default we compile with ThinLTO if the module has a summary, but the
   // client can request full LTO with a module flag.
   bool IsThinLTO = true;
   if (auto *MD =
void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   // Abbrev for FS_PERMODULE_PROFILE.
   auto Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_PROFILE));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // instcount
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // fflags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // numrefs
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // immutablerefcnt
   // numrefs x valueid, n x (valueid, hotness)
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSCallsProfileAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_PERMODULE or FS_PERMODULE_RELBF.
   Abbv = std::make_shared<BitCodeAbbrev>();
   if (WriteRelBFToSummary)
     Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_RELBF));
   else
     Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // instcount
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // fflags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // numrefs
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // immutablerefcnt
   // numrefs x valueid, n x (valueid [, rel_block_freq])
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSCallsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_PERMODULE_GLOBALVAR_INIT_REFS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE_GLOBALVAR_INIT_REFS));
void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   auto Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // modid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // instcount
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // fflags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // numrefs
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // immutablerefcnt
   // numrefs x valueid, n x (valueid)
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSCallsAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_COMBINED_PROFILE.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED_PROFILE));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // valueid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // modid
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // flags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));   // instcount
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // fflags
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // numrefs
+  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4));   // immutablerefcnt
   // numrefs x valueid, n x (valueid, hotness)
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));
   unsigned FSCallsProfileAbbrev = Stream.EmitAbbrev(std::move(Abbv));
 
   // Abbrev for FS_COMBINED_GLOBALVAR_INIT_REFS.
   Abbv = std::make_shared<BitCodeAbbrev>();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_COMBINED_GLOBALVAR_INIT_REFS));
if (auto *AS = dyn_cast<AliasSummary>(S)) {
       Aliases.push_back(AS);
       return;
     }
 
     if (auto *VS = dyn_cast<GlobalVarSummary>(S)) {
       NameVals.push_back(*ValueId);
       NameVals.push_back(Index.getModuleId(VS->modulePath()));
       NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
+      NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
       for (auto &RI : VS->refs()) {
         auto RefValueId = getValueId(RI.getGUID());
         if (!RefValueId)
           continue;
         NameVals.push_back(*RefValueId);
       }
 
       // Emit the finished record.
forEachSummary([&](GVInfo I, bool IsAliasee) {
     getReferencedTypeIds(FS, ReferencedTypeIds);
 
     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);
+    NameVals.push_back(0); // numrefs
+    NameVals.push_back(0); // immutablerefcnt
 
-    unsigned Count = 0;
+    unsigned Count = 0, ImmutableRefCnt = 0;
     for (auto &RI : FS->refs()) {
       auto RefValueId = getValueId(RI.getGUID());
       if (!RefValueId)
         continue;
       NameVals.push_back(*RefValueId);
+      if (RI.isReadOnly())
+        ImmutableRefCnt++;
       Count++;
     }
     NameVals[5] = Count;
+    NameVals[6] = ImmutableRefCnt;
 
     bool HasProfileData = false;
     for (auto &EI : FS->calls()) {
       HasProfileData |=
           EI.second.getHotness() != CalleeInfo::HotnessType::Unknown;
       if (HasProfileData)
         break;
     }

llvm/trunk/lib/IR/ModuleSummaryIndex.cpp

bool ValueInfo::isDSOLocal() const {
   // Need to check all summaries are local in case of hash collisions.
   return getSummaryList().size() &&
          llvm::all_of(getSummaryList(),
                       [](const std::unique_ptr<GlobalValueSummary> &Summary) {
                         return Summary->isDSOLocal();
                       });
 }
 
+// Gets the number of immutable refs in RefEdgeList
+unsigned FunctionSummary::immutableRefCount() const {
+  // Here we take advantage of having all readonly references
+  // located in the end of the RefEdgeList.
+  auto Refs = refs();
+  unsigned ImmutableRefCnt = 0;
+  for (int I = Refs.size() - 1; I >= 0 && Refs[I].isReadOnly(); --I)
+    ImmutableRefCnt++;
+  return ImmutableRefCnt;
+}
+
 // 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());
bool ModuleSummaryIndex::isGUIDLive(GlobalValue::GUID GUID) const {
   if (SummaryList.empty())
     return true;
   for (auto &I : SummaryList)
     if (isGlobalValueLive(I.get()))
       return true;
   return false;
 }
 
+static void propagateConstantsToRefs(GlobalValueSummary *S) {
+  // If reference is not readonly then referenced summary is not
+  // readonly either. Note that:
+  // - All references from GlobalVarSummary are conservatively considered as
+  //   not readonly. Tracking them properly requires more complex analysis
+  //   then we have now.
+  //
+  // - AliasSummary objects have no refs at all so this function is a no-op
+  //   for them.
+  for (auto &VI : S->refs()) {
+    if (VI.isReadOnly()) {
+      // We only mark refs as readonly when computing function summaries on
+      // analysis phase.
+      assert(isa<FunctionSummary>(S));
+      continue;
+    }
+    for (auto &Ref : VI.getSummaryList())
+      // If references to alias is not readonly then aliasee is not readonly
+      if (auto *GVS = dyn_cast<GlobalVarSummary>(Ref->getBaseObject()))
+        GVS->setReadOnly(false);
+  }
+}
+
+// Do the constant propagation in combined index.
+// The goal of constant propagation is internalization of readonly
+// variables. To determine which variables are readonly and which
+// are not we take following steps:
+// - During analysis we speculatively assign readonly attribute to
+//   all variables which can be internalized. When computing function
+//   summary we also assign readonly attribute to a reference if
+//   function doesn't modify referenced variable.
+//
+// - After computing dead symbols in combined index we do the constant
+//   propagation. During this step we clear readonly attribute from
+//   all variables which:
+//   a. are dead, preserved or can't be imported
+//   b. referenced by any global variable initializer
+//   c. referenced by a function and reference is not readonly
+//
+// Internalization itself happens in the backend after import is finished
+// See internalizeImmutableGVs.
+void ModuleSummaryIndex::propagateConstants(
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) {
+  for (auto &P : *this)
+    for (auto &S : P.second.SummaryList) {
+      if (!isGlobalValueLive(S.get()))
+        // We don't examine references from dead objects
+        continue;
+
+      // 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. It also 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).
+      if (auto *GVS = dyn_cast<GlobalVarSummary>(S->getBaseObject()))
+        // Here we intentionally pass S.get() not GVS, because S could be
+        // an alias.
+        if (!canImportGlobalVar(S.get()) || GUIDPreservedSymbols.count(P.first))
+          GVS->setReadOnly(false);
+      propagateConstantsToRefs(S.get());
+    }
+}
+
 // 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();
   }
 }
static void defineExternalNode(raw_ostream &OS, const char *Pfx,
   if (VI) {
     OS << getNodeVisualName(VI);
   } else {
     OS << getNodeVisualName(Id);
   }
   OS << "\"]; // defined externally\n";
 }
 
+static bool hasReadOnlyFlag(const GlobalValueSummary *S) {
+  if (auto *GVS = dyn_cast<GlobalVarSummary>(S))
+    return GVS->isReadOnly();
+  return false;
+}
+
 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, uint64_t SrcMod, GlobalValue::GUID SrcId,
-                      uint64_t DstMod, GlobalValue::GUID DstId, int TypeOrHotness) {
-    // 0 corresponds to alias edge, 1 to ref edge, 2 to call with unknown
-    // hotness, ...
-    TypeOrHotness += 2;
+                      uint64_t DstMod, GlobalValue::GUID DstId,
+                      int TypeOrHotness) {
+    // 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 && hasReadOnlyFlag(SummaryIt.second))
+          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(), R.isReadOnly() ? -1 : -2);
 
       if (auto *AS = dyn_cast_or_null<AliasSummary>(SummaryIt.second)) {
         GlobalValue::GUID AliaseeId;
         if (AS->hasAliaseeGUID())
           AliaseeId = AS->getAliaseeGUID();
         else {
           auto AliaseeOrigId = AS->getAliasee().getOriginalName();
           AliaseeId = getGUIDFromOriginalID(AliaseeOrigId);
           if (!AliaseeId)
             AliaseeId = AliaseeOrigId;
         }
 
-        Draw(SummaryIt.first, AliaseeId, -2);
+        Draw(SummaryIt.first, AliaseeId, -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));
     }

llvm/trunk/lib/LTO/LTO.cpp

#endif
 
   auto AddUsedThings = [&](GlobalValueSummary *GS) {
     if (!GS) return;
     AddUnsigned(GS->isLive());
     for (const ValueInfo &VI : GS->refs()) {
       AddUnsigned(VI.isDSOLocal());
       AddUsedCfiGlobal(VI.getGUID());
     }
+    if (auto *GVS = dyn_cast<GlobalVarSummary>(GS))
+      AddUnsigned(GVS->isReadOnly());
     if (auto *FS = dyn_cast<FunctionSummary>(GS)) {
       for (auto &TT : FS->type_tests())
         UsedTypeIds.insert(TT);
       for (auto &TT : FS->type_test_assume_vcalls())
         UsedTypeIds.insert(TT.GUID);
       for (auto &TT : FS->type_checked_load_vcalls())
         UsedTypeIds.insert(TT.GUID);
       for (auto &TT : FS->type_test_assume_const_vcalls())
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, Conf.OptLevel > 0);
 
   // 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);

llvm/trunk/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,
+                                  /* ImportEnabled = */ true);
 }
 
 /**
  * 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) {

llvm/trunk/lib/Linker/IRMover.cpp

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

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

if (DefinedGVSummaries.count(VI.getGUID())) {
       LLVM_DEBUG(
           dbgs() << "Ref ignored! Target already in destination module.\n");
       continue;
     }
 
     LLVM_DEBUG(dbgs() << " ref -> " << VI << "\n");
 
     for (auto &RefSummary : VI.getSummaryList())
-      if (RefSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
-          !RefSummary->notEligibleToImport() &&
-          !GlobalValue::isInterposableLinkage(RefSummary->linkage()) &&
-          RefSummary->refs().empty()) {
+      if (isa<GlobalVarSummary>(RefSummary.get()) &&
+          canImportGlobalVar(RefSummary.get())) {
         auto ILI = ImportList[RefSummary->modulePath()].insert(VI.getGUID());
         // Only update stat if we haven't already imported this variable.
         if (ILI.second)
           NumImportedGlobalVarsThinLink++;
         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,
+    bool ImportEnabled) {
+  computeDeadSymbols(Index, GUIDPreservedSymbols, isPrevailing);
+  if (ImportEnabled) {
+    Index.propagateConstants(GUIDPreservedSymbols);
+  } else {
+    // If import is disabled we should drop read-only attribute
+    // from all summaries to prevent internalization.
+    for (auto &P : Index)
+      for (auto &S : P.second.SummaryList)
+        if (auto *GVS = dyn_cast<GlobalVarSummary>(S.get()))
+          GVS->setReadOnly(false);
+  }
+}
+
 /// 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.
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;
 }
 
+// Internalize values that we marked with specific attribute
+// in processGlobalForThinLTO.
+static void internalizeImmutableGVs(Module &M) {
+  for (auto &GV : M.globals()) {
+    // Skip GVs which have been converted to declarations
+    // by dropDeadSymbols.
+    if (GV.isDeclaration())
+      continue;
+    if (auto *GVar = dyn_cast<GlobalVariable>(&GV))
+      if (GVar->hasAttribute("thinlto-internalize")) {
+        GVar->setLinkage(GlobalValue::InternalLinkage);
+        GVar->setVisibility(GlobalValue::DefaultVisibility);
+      }
+  }
+}
+
 // 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;
 
if (Mover.move(std::move(SrcModule), GlobalsToImport.getArrayRef(),
                    [](GlobalValue &, IRMover::ValueAdder) {},
                    /*IsPerformingImport=*/true))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += GlobalsToImport.size();
     NumImportedModules++;
   }
 
+  internalizeImmutableGVs(DestModule);
+
   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 "
                     << DestModule.getModuleIdentifier() << "\n");
   return ImportedCount;
 }
 
 static bool doImportingForModule(Module &M) {
   if (SummaryFile.empty())
     report_fatal_error("error: -function-import requires -summary-file\n");
   Expected<std::unique_ptr<ModuleSummaryIndex>> IndexPtrOrErr =
-      getModuleSummaryIndexForFile(SummaryFile);
+      getModuleSummaryIndexForFile(SummaryFile);  
   if (!IndexPtrOrErr) {
     logAllUnhandledErrors(IndexPtrOrErr.takeError(), errs(),
                           "Error loading file '" + SummaryFile + "': ");
     return false;
   }
   std::unique_ptr<ModuleSummaryIndex> Index = std::move(*IndexPtrOrErr);
 
   // First step is collecting the import list.

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

FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV,
 
   llvm_unreachable("unknown linkage type");
 }
 
 void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
 
   // Check the summaries to see if the symbol gets resolved to a known local
   // definition.
+  ValueInfo VI;
   if (GV.hasName()) {
-    ValueInfo VI = ImportIndex.getValueInfo(GV.getGUID());
+    VI = ImportIndex.getValueInfo(GV.getGUID());
     if (VI && VI.isDSOLocal()) {
       GV.setDSOLocal(true);
       if (GV.hasDLLImportStorageClass())
         GV.setDLLStorageClass(GlobalValue::DefaultStorageClass);
     }
   }
 
+  // Mark read-only variables which can be imported with specific attribute.
+  // We can't internalize them now because IRMover will fail to link variable
+  // definitions to their external declarations during ThinLTO import. We'll
+  // internalize read-only variables later, after import is finished.
+  // See internalizeImmutableGVs.
+  //
+  // If global value dead stripping is not enabled in summary then
+  // propagateConstants hasn't been run (may be because we're using
+  // distriuted import. We can't internalize GV in such case.
+  if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {
+    const auto &SL = VI.getSummaryList();
+    auto *GVS = SL.empty() ? nullptr : dyn_cast<GlobalVarSummary>(SL[0].get());
+    if (GVS && GVS->isReadOnly())
+      cast<GlobalVariable>(&GV)->addAttribute("thinlto-internalize");
+  }
+
   bool DoPromote = false;
   if (GV.hasLocalLinkage() &&
       ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) {
     // Once we change the name or linkage it is difficult to determine
     // again whether we should promote since shouldPromoteLocalToGlobal needs
     // to locate the summary (based on GUID from name and linkage). Therefore,
     // use DoPromote result saved above.
     GV.setName(getName(&GV, DoPromote));
     GV.setLinkage(getLinkage(&GV, DoPromote));
     if (!GV.hasLocalLinkage())
       GV.setVisibility(GlobalValue::HiddenVisibility);
   } else
     GV.setLinkage(getLinkage(&GV, /* DoPromote */ false));
 
   // Remove functions imported as available externally defs from comdats,
   // as this is a declaration for the linker, and will be dropped eventually.
   // It is illegal for comdats to contain declarations.
-  auto *GO = dyn_cast_or_null<GlobalObject>(&GV);
+  auto *GO = dyn_cast<GlobalObject>(&GV);
   if (GO && GO->isDeclarationForLinker() && GO->hasComdat()) {
     // The IRMover should not have placed any imported declarations in
     // a comdat, so the only declaration that should be in a comdat
     // at this point would be a definition imported as available_externally.
     assert(GO->hasAvailableExternallyLinkage() &&
            "Expected comdat on definition (possibly available external)");
     GO->setComdat(nullptr);
   }

llvm/trunk/test/Bitcode/summary_version.ll

 ; Check summary versioning
 ; RUN: opt  -module-summary  %s -o - | llvm-bcanalyzer -dump | FileCheck %s
 
 ; CHECK: <GLOBALVAL_SUMMARY_BLOCK
-; CHECK: <VERSION op0=4/>
+; CHECK: <VERSION op0=5/>
 
 
 
 ; Need a function for the summary to be populated.
 define void @foo() {
     ret void
 }

llvm/trunk/test/Bitcode/thinlto-alias.ll

 ; "main"
 ; CHECK-NEXT: <FUNCTION op0=0 op1=4
 ; "analias"
 ; CHECK-NEXT: <FUNCTION op0=4 op1=7
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered.
 ; The value id 1 matches the second FUNCTION record above.
-; CHECK-NEXT:    <PERMODULE {{.*}} op5=1/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op6=1/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'mainanalias{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS
 ; See if the call to analias is registered, using the expected value id.
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[ALIASID:[0-9]+]] op1=-5751648690987223394/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[ALIASEEID:[0-9]+]] op1=-1039159065113703048/>
-; COMBINED-NEXT:    <COMBINED {{.*}} op6=[[ALIASID]]/>
+; COMBINED-NEXT:    <COMBINED {{.*}} op7=[[ALIASID]]/>
 ; COMBINED-NEXT:    <COMBINED {{.*}}
 ; COMBINED-NEXT:    <COMBINED_ALIAS  {{.*}} op3=[[ALIASEEID]]
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

llvm/trunk/test/Bitcode/thinlto-alias2.ll

 ; Test to check the callgraph for call to alias in module.
 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
 
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
-; CHECK-NEXT:    <PERMODULE {{.*}} op4=0 op5=[[ALIASID:[0-9]+]]/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op4=0 op5=0 op6=[[ALIASID:[0-9]+]]/>
 ; CHECK-NEXT:    <PERMODULE {{.*}} op0=[[ALIASEEID:[0-9]+]]
 ; CHECK-NEXT:    <ALIAS {{.*}} op0=[[ALIASID]] {{.*}} op2=[[ALIASEEID]]/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; ModuleID = 'thinlto-alias2.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph-cast.ll

 ; Test to check the callgraph for calls to casts.
 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
 ; PR34966
 
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; "op7" is a call to "callee" function.
-; CHECK-NEXT:    <PERMODULE {{.*}} op7=3 op8=[[ALIASID:[0-9]+]]/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op8=3 op9=[[ALIASID:[0-9]+]]/>
 ; "another_caller" has only references but no calls.
-; CHECK-NEXT:    <PERMODULE {{.*}} op4=3 {{.*}} op7={{[0-9]+}}/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op4=3 {{.*}} op8={{[0-9]+}}/>
 ; CHECK-NEXT:    <PERMODULE {{.*}} op0=[[ALIASEEID:[0-9]+]]
 ; CHECK-NEXT:    <ALIAS {{.*}} op0=[[ALIASID]] {{.*}} op2=[[ALIASEEID]]/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; ModuleID = 'thinlto-function-summary-callgraph-cast.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph-pgo.ll

 
 ; CHECK: <SOURCE_FILENAME
 ; CHECK-NEXT: <FUNCTION
 ; "func"
 ; CHECK-NEXT: <FUNCTION op0=4 op1=4
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered, using the expected hotness type.
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op5=1 op6=2/>
+; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op6=1 op7=2/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'mainfunc{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[FUNCID:[0-9]+]] op1=7289175272376759421/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED
 ; See if the call to func is registered, using the expected hotness type.
 ; op6=2 which is hotnessType::None.
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op6=[[FUNCID]] op7=2/>
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op7=[[FUNCID]] op8=2/>
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() #0 !prof !2 {

llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph-profile-summary.ll

 ; "none2"
 ; CHECK-NEXT: <FUNCTION op0=37 op1=5
 ; "none3"
 ; CHECK-NEXT: <FUNCTION op0=42 op1=5
 ; CHECK-LABEL:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; CHECK-NEXT:    <VALUE_GUID op0=25 op1=123/>
 ; op4=hot1 op6=cold op8=hot2 op10=hot4 op12=none1 op14=hot3 op16=none2 op18=none3 op20=123
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op5=1 op6=3 op7=5 op8=1 op9=2 op10=3 op11=4 op12=1 op13=6 op14=2 op15=3 op16=3 op17=7 op18=2 op19=8 op20=2 op21=25 op22=4/>
+; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op6=1 op7=3 op8=5 op9=1 op10=2 op11=3 op12=4 op13=1 op14=6 op15=2 op16=3 op17=3 op18=7 op19=2 op20=8 op21=2 op22=25 op23=4/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'hot_functionhot1hot2hot3hot4coldnone1none2none3{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op6=[[HOT1:.*]] op7=3 op8=[[COLD:.*]] op9=1 op10=[[HOT2:.*]] op11=3 op12=[[NONE1:.*]] op13=2 op14=[[HOT3:.*]] op15=3 op16=[[NONE2:.*]] op17=2 op18=[[NONE3:.*]] op19=2/>
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op7=[[HOT1:.*]] op8=3 op9=[[COLD:.*]] op10=1 op11=[[HOT2:.*]] op12=3 op13=[[NONE1:.*]] op14=2 op15=[[HOT3:.*]] op16=3 op17=[[NONE2:.*]] op18=2 op19=[[NONE3:.*]] op20=2/>
 ; COMBINED_NEXT:    <COMBINED abbrevid=
 ; COMBINED_NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

llvm/trunk/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:    <PERMODULE_RELBF {{.*}} op4=1 {{.*}} op7=256
+; CHECK-NEXT:    <PERMODULE_RELBF {{.*}} op4=1 {{.*}} op8=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"
 target triple = "x86_64-unknown-linux-gnu"

llvm/trunk/test/Bitcode/thinlto-function-summary-callgraph-sample-profile-summary.ll

 ; "none3"
 ; CHECK-NEXT: <FUNCTION op0=44 op1=5
 ; CHECK-NEXT: <FUNCTION op0=49 op1=5
 
 ; CHECK-LABEL:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; CHECK-NEXT:    <VALUE_GUID op0=26 op1=123/>
 ; op4=none1 op6=hot1 op8=cold1 op10=none2 op12=hot2 op14=cold2 op16=none3 op18=hot3 op20=cold3 op22=123
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op5=7 op6=0 op7=1 op8=3 op9=4 op10=1 op11=8 op12=0 op13=2 op14=3 op15=5 op16=1 op17=9 op18=0 op19=3 op20=3 op21=6 op22=1 op23=26 op24=4/>
+; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op6=7 op7=0 op8=1 op9=3 op10=4 op11=1 op12=8 op13=0 op14=2 op15=3 op16=5 op17=1 op18=9 op19=0 op20=3 op21=3 op22=6 op23=1 op24=26 op25=4/>
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'hot_functionhot1hot2hot3cold1cold2cold3none1none2none3{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS

 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op6=[[NONE1:.*]] op7=0 op8=[[HOT1:.*]] op9=3 op10=[[COLD1:.*]] op11=1 op12=[[NONE2:.*]] op13=0 op14=[[HOT2:.*]] op15=3 op16=[[COLD2:.*]] op17=1 op18=[[NONE3:.*]] op19=0 op20=[[HOT3:.*]] op21=3 op22=[[COLD3:.*]] op23=1/>
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op7=[[NONE1:.*]] op8=0 op9=[[HOT1:.*]] op10=3 op11=[[COLD1:.*]] op12=1 op13=[[NONE2:.*]] op14=0 op15=[[HOT2:.*]] op16=3 op17=[[COLD2:.*]] op18=1 op19=[[NONE3:.*]] op20=0 op21=[[HOT3:.*]] op22=3 op23=[[COLD3:.*]] op24=1/>
 ; COMBINED_NEXT:    <COMBINED abbrevid=
 ; COMBINED_NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

llvm/trunk/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:    <PERMODULE {{.*}} op4=1
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'undefinedglobmainfunc{{.*}}'
 
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; COMBINED-NEXT:    <FLAGS
 ; Only 2 VALUE_GUID since reference to undefinedglob should not be included in
 ; combined index.
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[FUNCID:[0-9]+]] op1=7289175272376759421/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED
 ; See if the call to func is registered.
-; COMBINED-NEXT:    <COMBINED {{.*}} op6=[[FUNCID]]/>
+; COMBINED-NEXT:    <COMBINED {{.*}} op7=[[FUNCID]]/>
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() #0 {

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

 ; for calls). Use different linkage types for the various test cases to
 ; distinguish the test cases here (op1 contains the linkage type).
 ; Note that op3 contains the # non-call references.
 ; This also ensures that we didn't include a call or reference to intrinsic
 ; llvm.ctpop.i8.
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; Function main contains call to func, as well as address reference to func:
 ; op0=main op4=func op5=func
-; CHECK-DAG:    <PERMODULE {{.*}} op0=11 op1=0 {{.*}} op4=1 op5=2 op6=2/>
+; CHECK-DAG:    <PERMODULE {{.*}} op0=11 op1=0 {{.*}} op4=1 op5=0 op6=2 op7=2/>
 ; Function W contains a call to func3 as well as a reference to globalvar:
 ; op0=W op4=globalvar op5=func3
-; CHECK-DAG:    <PERMODULE {{.*}} op0=6 op1=5 {{.*}} op4=1 op5=1 op6=5/>
+; CHECK-DAG:    <PERMODULE {{.*}} op0=6 op1=5 {{.*}} op4=1 op5=0 op6=1 op7=5/>
 ; Function X contains call to foo, as well as address reference to foo
 ; which is in the same instruction as the call:
 ; op0=X op4=foo op5=foo
-; CHECK-DAG:    <PERMODULE {{.*}} op0=7 op1=1 {{.*}} op4=1 op5=4 op6=4/>
+; CHECK-DAG:    <PERMODULE {{.*}} op0=7 op1=1 {{.*}} op4=1 op5=0 op6=4 op7=4/>
 ; Function Y contains call to func2, and ensures we don't incorrectly add
 ; a reference to it when reached while earlier analyzing the phi using its
 ; return value:
 ; op0=Y op4=func2
-; CHECK-DAG:    <PERMODULE {{.*}} op0=8 op1=72 {{.*}} op4=0 op5=3/>
+; CHECK-DAG:    <PERMODULE {{.*}} op0=8 op1=72 {{.*}} op4=0 op5=0 op6=3/>
 ; Function Z contains call to func2, and ensures we don't incorrectly add
 ; a reference to it when reached while analyzing subsequent use of its return
 ; value:
 ; op0=Z op4=func2
-; CHECK-DAG:    <PERMODULE {{.*}} op0=9 op1=3 {{.*}} op4=0 op5=3/>
+; CHECK-DAG:    <PERMODULE {{.*}} op0=9 op1=3 {{.*}} op4=0 op5=0 op6=3/>
 ; Variable bar initialization contains address reference to func:
 ; op0=bar op2=func
-; CHECK-DAG:    <PERMODULE_GLOBALVAR_INIT_REFS {{.*}} op0=0 op1=0 op2=2/>
+; CHECK-DAG:    <PERMODULE_GLOBALVAR_INIT_REFS {{.*}} op0=0 op1=0 op2=1 op3=2/>
 ; CHECK:  </GLOBALVAL_SUMMARY_BLOCK>
 
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'barglobalvarfuncfunc2foofunc3WXYZllvm.ctpop.i8main{{.*}}'
 
 ; ModuleID = 'thinlto-function-summary-refgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-alias.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = global i32 42, align 4
[e-mail] = weak alias i32, i32* @g

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-comdat.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$comdat.any = comdat any
+@g = global i32 42, comdat($comdat.any)

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-define-g.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = global i32 42, align 4

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-full-lto.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = external global i32
+
+define i32 @foo() {
+  %v = load i32, i32* @g
+  ret i32 %v
+}
+
+!0 = !{i32 1, !"ThinLTO", i32 0}
+!llvm.module.flags = !{ !0 }

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-gvref.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@b = global i32* @a, align 8
+@a = global i32 42, align 4

llvm/trunk/test/ThinLTO/X86/Inputs/index-const-prop-linkage.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g1 = common global i32 0, align 4
+@g2 = global i32 42, align 4
+@g3 = available_externally global i32 42, align 4
+
+define i32 @foo() {
+  %v1 = load i32, i32* @g1
+  %v2 = load i32, i32* @g2
+  %v3 = load i32, i32* @g3
+  %s1 = add i32 %v1, %v2
+  %s2 = add i32 %s1, %v3
+  ret i32 %s2
+}

llvm/trunk/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)

llvm/trunk/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{{.*}} ffl: 00001{{.*}}]; // function
-; 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

llvm/trunk/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-lto -thinlto-action=import -exported-symbol=main %t1.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
 }

llvm/trunk/test/ThinLTO/X86/index-const-prop-O0.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-define-g.ll -o %t2.bc
+; RUN: llvm-lto2 run -O0 -save-temps %t2.bc -r=%t2.bc,g,pl %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,g, -o %t3
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s
+
+; With -O0 import is disabled so we must not internalize
+; read-only globals
+; CHECK: @g = dso_local global i32 42
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = external global i32
+
+define i32 @main() {
+  %v = load i32, i32* @g
+  ret i32 %v
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-alias.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-alias.ll -o %t2.bc
+; RUN: llvm-lto2 run %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,ret_ptr,pl -r=%t1.bc,g.alias,l -r=%t1.bc,g,l \
+; RUN:               %t2.bc -r=%t2.bc,g,pl -r=%t2.bc,g.alias,pl -save-temps -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
+
+; When ret_ptr is preserved we return pointer to alias, so we can't internalize aliasee
+; RUN: llvm-lto2 run %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,ret_ptr,plx -r=%t1.bc,g.alias,l -r=%t1.bc,g,l \
+; RUN:               %t2.bc -r=%t2.bc,g,pl -r=%t2.bc,g.alias,pl -save-temps -o %t4
+; RUN: llvm-dis %t4.1.3.import.bc -o - | FileCheck %s --check-prefix=PRESERVED
+
+; When g.alias is preserved we can't internalize aliasee either
+; RUN: llvm-lto2 run %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,ret_ptr,pl -r=%t1.bc,g.alias,l -r=%t1.bc,g,l \
+; RUN:               %t2.bc -r=%t2.bc,g,pl -r=%t2.bc,g.alias,plx -save-temps -o %t5
+; RUN: llvm-dis %t5.1.3.import.bc -o - | FileCheck %s --check-prefix=PRESERVED
+
+; We currently don't support importing aliases
+; IMPORT:       @g.alias = external dso_local global i32
+; IMPORT-NEXT:  @g = internal global i32 42, align 4 #0
+; IMPORT:  attributes #0 = { "thinlto-internalize" }
+
+; CODEGEN:      define dso_local i32 @main
+; CODEGEN-NEXT:    ret i32 42
+
+; PRESERVED:      @g.alias = external dso_local global i32
+; PRESERVED-NEXT: @g = available_externally dso_local global i32 42, align 4
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
[e-mail] = external global i32
+@g = external global i32
+
+define i32 @main() {
+  %v = load i32, i32* @g
+  ret i32 %v
+}
+
+define i32* @ret_ptr() {
+  ret i32* @g.alias
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-comdat.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-comdat.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,g,pl %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,g, -o %t3
+; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
+
+; Comdats are not internalized even if they are read only.
+; CHECK: @g = available_externally dso_local global i32 42 
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = external global i32
+
+define i32 @main() {
+  %v = load i32, i32* @g
+  ret i32 %v
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-dead.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-define-g.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,g,pl \
+; RUN:               %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,foo,pl -r=%t1.bc,g, -o %t3
+; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
+
+; Dead globals are converted to declarations by ThinLTO in dropDeadSymbols
+; If we try to internalize such we'll get a broken module. 
+; CHECK: @g = external dso_local global i32
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = external global i32
+
+; We need at least one live symbol to enable dead stripping
+; Otherwise ModuleSummaryIndex::isGlobalValueLive will always
+; return true.
+define i32 @main() {
+  ret i32 42
+}
+
+define i32 @foo() {
+  %v = load i32, i32* @g
+  ret i32 %v
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-full-lto.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-define-g.ll -o %t2.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-full-lto.ll -o %t3.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,g,pl \
+; RUN:                 %t1.bc -r=%t1.bc,foo,l -r=%t1.bc,main,plx -r=%t1.bc,g, \
+; RUN:                 %t3.bc -r=%t3.bc,foo,pl -r=%t3.bc,g, -o %t4
+; RUN: llvm-dis %t4.2.3.import.bc -o - | FileCheck %s
+
+; All references from functions in full LTO module are not constant.
+; We cannot internalize @g
+; CHECK: @g = available_externally dso_local global i32 42
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @foo()
+@g = external global i32
+
+define i32 @main() {
+  %v = call i32 @foo()
+  %v2 = load i32, i32* @g
+  %v3 = add i32 %v, %v2
+  ret i32 %v3
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-gvref.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-gvref.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,b,pl -r=%t2.bc,a,pl \
+; RUN:               %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,a, -r=%t1.bc,b, -o %t3
+; RUN: llvm-dis %t3.1.3.import.bc -o - | FileCheck %s --check-prefix=SRC
+; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s --check-prefix=DEST
+
+; No variable in the source module should have been internalized
+; SRC:      @b = dso_local global i32* @a
+; SRC-NEXT: @a = dso_local global i32 42
+
+; We can't internalize globals referenced by other live globals
+; DEST:      @b = external dso_local global i32*
+; DEST-NEXT: @a = available_externally dso_local global i32 42, align 4
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = external global i32
+@b = external global i32*
+
+define i32 @main() {
+  %p = load i32*, i32** @b, align 8  
+  store i32 33, i32* %p, align 4
+  %v = load i32, i32* @a, align 4
+  ret i32 %v
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-ldst.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-define-g.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,g,pl %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,g, -o %t3
+; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
+
+; The 'store' instruction in @main should prevent internalization
+; even when there is 'load' instruction before it.
+; CHECK: @g = available_externally dso_local global i32 42
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@g = external global i32
+
+define i32 @main() {
+  %v = load i32, i32* @g
+  %q = add i32 %v, 1
+  store i32 %q, i32* @g
+  
+  ret i32 %v
+}

llvm/trunk/test/ThinLTO/X86/index-const-prop-linkage.ll

+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/index-const-prop-linkage.ll -o %t2.bc
+; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,foo,pl -r=%t2.bc,g1,pl -r=%t2.bc,g2,pl -r=%t2.bc,g3, \
+; RUN:                           %t1.bc -r=%t1.bc,foo, -r=%t1.bc,main,plx -r=%t1.bc,g2,  -o %t3
+; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
+
+; Check that we never internalize anything with:
+; - appending linkage
+; - common linkage
+; - available_externally linkage
+; - reference from @llvm.used
+; CHECK:      @llvm.used = appending global [1 x i32*] [i32* @g2]
+; CHECK-NEXT: @g1 = external dso_local global i32, align 4
+; CHECK-NEXT: @g2 = available_externally dso_local global i32 42, align 4
+; CHECK-NEXT: @g3 = available_externally global i32 42, align 4
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i32 @foo()
+@g2 = external global i32
[e-mail] = appending global [1 x i32*] [i32* @g2]
+
+define i32 @main() {
+  %v = call i32 @foo()
+  ret i32 %v
+}

llvm/trunk/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 -exported-symbol=main  %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"
+
+@gBar = external global i32
+
+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

llvm/trunk/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:  -r=%t1.bc,gBar, \
+; 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:  -r=%t1.bc,gBar, \
+; 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"
+
+; We should be able to link external definition of gBar to its declaration
+@gBar = external global i32
+
+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