[PATCH] D109792: Resolve {GlobalValue,GloalIndirectSymol}::getBaseObject confusion
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list