[PATCH] D65115: IR: Teach GlobalIndirectSymbol::getBaseObject() to handle more kinds of expressions.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked 3 inline comments as done.
pcc added inline comments.


================
Comment at: llvm/lib/IR/Globals.cpp:436
+  if (auto *GIS = dyn_cast<GlobalIndirectSymbol>(C))
+    if (Aliases.insert(GIS).second)
+      return findBaseObject(GIS->getOperand(0), Aliases);
----------------
tejohnson wrote:
> Why is this needed? Is it possible to have cycles in the indirection?
Yes, in invalid IR. Without this we hang when reading this IR instead of erroring in the verifier. See `llvm/test/Verifier/alias.ll` for one example.


================
Comment at: llvm/lib/IR/Globals.cpp:441
+    case Instruction::Add: {
+      if (auto *GIS = findBaseObject(CE->getOperand(0), Aliases))
+        return GIS;
----------------
tejohnson wrote:
> Is it possible for the other operand of the add (or subtract below) to be something that disqualifies this from analysis,  i.e. that needs checking?
Yes, `A+B` where both `A` and `B` refer to GlobalObjects would mean we need to return nullptr here. I'll add a case for that. For subtract we're already checking both operands.


================
Comment at: llvm/test/Linker/comdat8.ll:6
+ at some_name = unnamed_addr constant i32 42, comdat($c1)
+ at c1 = alias i8, inttoptr (i32 1 to i8*)
 
----------------
tejohnson wrote:
> Why does this test change?
Because now we can analyze the aliasee, so it needed to be replaced with something that doesn't refer to a base object. See http://llvm-cs.pcc.me.uk/lib/Linker/LinkModules.cpp#134 for the call that now succeeds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65115/new/

https://reviews.llvm.org/D65115