[patch][PR10367] Fix the design of GlobalAlias

[Rafael Espíndola]

On 13 May 2014 15:47, Reid Kleckner <rnk at google.com> wrote:
> +// FIXME: Delete this in LLVM 4.0 and just assert that the aliasse is a
>
> "aliasee"  Chrome doesn't think it's a word, but our API has getAliasee, so
> let's go with it.

Fixed.

> +// GlobalObject.
> +static GlobalObject &
> +getGlobalObjectInExpr(const DenseMap<GlobalAlias *, Constant *> &Map,
> +                      Constant &C) {
> +  auto *GO = dyn_cast<GlobalObject>(&C);
> +  if (GO)
> +    return *GO;
> +  auto *GA = dyn_cast<GlobalAlias>(&C);
> +  if (GA)
> +    return getGlobalObjectInExpr(Map, *Map.find(GA)->second);
> +  auto &CE = cast<ConstantExpr>(C);
> +  assert(CE.getOpcode() == Instruction::BitCast ||
> +         CE.getOpcode() == Instruction::GetElementPtr ||
>
> We never supported GEPs to non-zero offsets, right?  Should we assert that
> or check that here?

assert added.

> +         CE.getOpcode() == Instruction::AddrSpaceCast);
> +  return getGlobalObjectInExpr(Map, *CE.getOperand(0));
> +}
>
>
> +  // FIXME: Delete this in LLVM 4.0
> +  // Older versions of llvm could write an alias pointing to another. We
> cannot
> +  // construct those aliases, so we first collect an alias to aliasee
> expression
> +  // and then compute the actual aliassee.
>
> "aliasee"

Fixed.

> +  DenseMap<GlobalAlias*, Constant *> AliasInit;
>
> +  for (auto &Pair : AliasInit) {
> +    setAliasProperties(AliasInit, *Pair.first, *Pair.second);
>
> This function is called once, I'd just inline it.

Done

> +  }
>
>
> -      GVar = dyn_cast_or_null<GlobalVariable>(GA->getAliasedGlobal());
> +      GVar = dyn_cast_or_null<GlobalVariable>(GA->getAliasee());
>
> This rename accounts for a large portion of the diff.  Is it worth
> committing separately?

Good point. I will split the patch.

I will post the patches as soon as I finish splitting them.

Cheers,
Rafael