[patch][PR10367] Fix the design of GlobalAlias

Reid Kleckner rnk at google.com
Tue May 13 12:47:29 PDT 2014


+// 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.

+// 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?

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

+  DenseMap<GlobalAlias*, Constant *> AliasInit;

+  for (auto &Pair : AliasInit) {
+    setAliasProperties(AliasInit, *Pair.first, *Pair.second);

This function is called once, I'd just inline it.

+  }


-      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?


On Mon, May 12, 2014 at 9:52 PM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> These patches change the design of GlobalAlias so that it doesn't take
> a ConstantExpr anymore.
>
> First, sorry for such large patches, but I am out of ideas on how to
> split them further. The patches are:
>
> * llvm-1.patch: Split GlobalValue into GlobalValue and GlobalObject.
> That allows us statically accept a Function or a GlobalVariable, but
> not an alias. This is already a cleanup by itself IMHO, but the main
> reason is that it gives a lot more confidence that the next patch is
> doing the right thing.
>
> * clang-1.patch is just an update for the api change.
>
> These two patches are already fairly mature. 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 '*'.
>
> 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.
>
> 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.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140513/321f66b9/attachment.html>


More information about the llvm-commits mailing list