[PATCH] D18298: ThinLTO: do not promote GlobalVariable that have a specific section.

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D18298#409219, @joker.eph wrote:

> In http://reviews.llvm.org/D18298#409215, @tejohnson wrote:
>
> > In http://reviews.llvm.org/D18298#409202, @joker.eph wrote:
> >
> > > So the only way I could foresee to make it work requires collision detection in names based on the summary. Of course it will require to some changes in the naming scheme (the complication comes from using hashes...).
> > >  The hash function is currently   `hash(symbol_name, linkage, module_id)` ; this is the GUID we store in the bitcode *and* we use to index in the global table in the combined index.
> > >
> > > We can instead compute another GUID this way: `hash(hash(symbol_name), hash(linkage, module_id)) `.
> > >  During the FE phase, we generate in the summary only this part: `hash(symbol_name)` ; this is what we store in the bitcode file. We already have `module_id` and `linkage` propagated somehow.
> > >  In the combined index in memory, we store `hash(symbol_name)` in the summary itself (`GlobalValueSummary`), and compute the finale GUID `hash(hash(symbol_name), hash(linkage, module_id))` for the key in the global table.
> > >  That way we can always lookup the summary of all the symbol in a module and compare `hash(symbol_name)` to detect the collision.
> >
> >
> > I think a variant of this should work. The current hashing scheme is actually `(IsLocalLinkage)?hash(SourceFileName+":"+symbol_name):hash(symbol_name)`
> >
> > We don't currently have the SourceFileName in the combined index. We could however augment the module path string table to include that for use in computing the GUID/hash key into the combined index table for locals. Then use your idea of keeping a hash of just the symbol name in the combined index summary for comparison,
>
>
> Peter was thinking about changing the naming scheme right? Instead of using the `SourceFileName` we would hash the names of all the public globals in a module IIRC. With a minor variant, by hashing the hash of each global name we don't need any other information in the bitcode to build the module_hash.


(Following up from IRC discussion). This is different than the way we do the promotion naming. I don't think this change affects or is related to the promotion naming scheme.

> 

> 

> > although we only really need to do that in the case where IsLocalLinkage==true (if !IsLocalLinkage then its  GUID/hash used as key in the index can be used for the comparison).

> 

> 

> Yes but any suggestion on how to achieve having this field optional? Unless having an extra layer in the hierarchy `LocalFunctionSummary` inheriting from `FunctionSummary` and `LocalAliasSummary` inheriting from `AliasSummary` etc. But this does not seem like the right design (won't scale with other fields).

>  Another way may be to have a variable size class that we malloc we extra size for the optional fields.


I was thinking more about the combined index format it bitcode, where we would want to emit as an optional record.


http://reviews.llvm.org/D18298