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

Tue Sep 21 13:17:08 PDT 2021

MaskRay added a comment.

Thanks:) Will push in two days.

In D109792#3013421 <https://reviews.llvm.org/D109792#3013421>, @ibookstein wrote:

> 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).

Inheriting from `GlobalObject` makes sense to me, since we won't use any facility from `GlobalIndirectSymbol`.

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

Makes sense to me.

> 1. 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.

Yes, I think removing `GlobalValue::getBaseObject()` will be worthwhile. My quick glance at the call sites suggest that they don't use the `GlobalIFunc::getResolverFunction()` facility.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109792