[PATCH] D109792: Resolve {GlobalValue,GloalIndirectSymol}::getBaseObject confusion

Itay Bookstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 21 12:58:37 PDT 2021


ibookstein accepted this revision.
ibookstein added a comment.
This revision is now accepted and ready to land.

I've added one additional minor comment; other than that, I think this makes good progress in resolving the ambiguity currently in trunk.
I still think that to be able to express alias-to-ifunc situations (i.e. `@foo1 = ifunc i32 (i32), i64 ()* @foo_resolver; @foo2 = alias i32 (i32), i32 (i32)* @foo1`) we'll need to consider https://reviews.llvm.org/D108872, but that's a problem for another day (and I'd appreciate your opinion on that direction).

A couple of questions:

1. This effectively turns `getBaseObject()` into `getAliaseeObject()`, so it might make sense to perform that rename in a later commit - WDYT?
2. If I understand you correctly, calls to `GlobalValue::getBaseObject()` are exceedingly rare. Would it make sense to try and remove it from the base class so that only `GlobalIFunc::getResolverFunction()` and `GlobalAlias::getAliaseeObject()` remain? I like the explicitness that that would create, but I'm wondering whether that sacrifices utility too much.



================
Comment at: llvm/lib/IR/Globals.cpp:467
 
-const GlobalObject *GlobalIndirectSymbol::getBaseObject() const {
-  DenseSet<const GlobalAlias *> Aliases;
----------------
Should also remove the declaration then, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109792



More information about the llvm-commits mailing list