[PATCH] D49362: [ThinLTO] Compute constant references

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/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.




https://reviews.llvm.org/D49362