[patch][PR10367] Fix the design of GlobalAlias

[Duncan P. N. Exon Smith]

On 2014-May-12, at 21:52, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

> The next two still need
> polishing and some bug fixes, but it should already be possible to
> provide feedback on some high level issues. In particular:
> 
> To avoid changing all alias related tests in these patches, I kept the
> common syntax
> 
> @foo = alias i32* @bar
> 
> to mean the same as now. The more general case when the types don't
> match is written
> 
> @foo = alias i16, i32* @bar.
> 
> Note that GlobalAlias now behaves a bit more like GlobalVariable. We
> know that its type is always a pointer, so we omit the '*'.

This syntax is a big improvement.

> For the bitcode, a nice surprise is that we were writing both
> identical types already, so the format change is minimal. Auto upgrade
> is handled by looking through the casts and no new fields are needed
> for now. New bitcode will simply have different types for Alias and
> Aliassee.
> 
> One last interesting point in the patch is that replaceAllUsesWith
> becomes smart enough to avoid putting a ConstantExpr in the aliassee.
> This seems better than checking and updating every caller.

+1

> What is still not implemented (and will be done in a followup patch)
> is to add an offset field, so that we can represent an alias that is
> inside another symbol.

I'm a little worried about the complexity of supporting the full power
of GEP here; it seems like a fair bit of duplicated logic.  Although,
maybe you're not planning to support all of it?  Or maybe it'll be
cleaner than I imagine.

I like this direction nevertheless.  This makes aliases much easier to
reason about.

I found a few of small things (clang-format level stuff) not worth
mentioning yet, but make sure you update this comment:

    /// ParseAlias:
    ///   ::= GlobalVar '=' OptionalVisibility OptionalDLLStorageClass 'alias'
    ///                     OptionalLinkage Aliasee
    /// Aliasee
    ///   ::= TypeAndValue
    ///   ::= 'bitcast' '(' TypeAndValue 'to' Type ')'
    ///   ::= 'getelementptr' 'inbounds'? '(' ... ')'
    bool LLParser::ParseAlias(const std::string &Name, LocTy NameLoc,