thr3ads.net - llvm dev - [llvm-dev] [IR] Modelling of GlobalIFunc [Sep 2021]

If this information is useful, please help other people find it:
Share via:

Itay Bookstein via llvm-dev

2021-Sep-11 10:21 UTC

[llvm-dev] [IR] Modelling of GlobalIFunc

That really depends on what the correct behavior is for
getBaseObject(). As far as I can tell, this method was originally
added to pierce through GlobalAliases (including various
ConstantExpr-s) and get to the "subject" *GlobalObject* (which is its
return type).

When looking at getBaseObject() through these
GlobalAlias-colored-glasses, I think that the following invariant
should follow:
* getBaseObject() should be idempotent (at least for the cases that it
does not return a nullptr): GV->getBaseObject()->getBaseObject()
=GV->getBaseObject().
Note that a GlobalAlias may have a different type than its base object
[4], so my previous arguments about a same-type-invariant are
incorrect.

Now, let us consider the case of a GlobalAlias where the aliasee is a
GlobalIFunc. This happens in glibc for example.
What should GA->getBaseObject() return in this case? I think there are
three options here:
1. nullptr; this sort-of-misses the original point of getBaseObject(),
since there *is* an aliasee, you don't even need to pierce through
ConstantExpr-s to get it.
2. The GlobalIFunc itself; this is only possible with either:
2. a. Making GlobalIFunc inherit from GlobalObject [1].
2. b. Changing the return type of getBaseObject() to be GlobalValue
(which also seems to be missing the original point of
getBaseObject()).
3. The GlobalIFunc's resolver; this is similar to the behavior of
getRootObject() in my PR [1].

The current situation on top of master is that GA->getBaseObject()
would return nullptr, but GI->getBaseObject() would return the
resolver, which makes it non-idempotent.
To resolve that, I think that either 2.a. or 3. make sense here.

Some users [2] consider the result of GlobalAlias::getBaseObject() to
be the aliasee [2], which option 3 doesn't uphold.
Other users [3] consider the result of getBaseObject() to be some
"root" / "key" / "leader" for a group of
GlobalValues that need to be
handled together, which option 2 doesn't uphold. This is the reason
for the getBaseObject/getRootObject split in my PR, which demonstrates
2.a.

There's also the replication of some of this type hierarchy and
semantics in {GlobalValue,Alias,Function,GlobalVar}Summary::getBaseObject(),
which I haven't yet considered. Note:
* IFunc is missing from that list
* GlobalValueSummary::getBaseObject() returns a GlobalValueSummary,
not a GlobalObjectSummary (which doesn't exist).

Thoughts?

[1] https://reviews.llvm.org/D108872
[2]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L4037
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp#L640
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L1675
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/LTO/LTO.cpp#L736
[3]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Transforms/Utils/SplitModule.cpp#L130
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Transforms/Utils/SplitModule.cpp#L229
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Object/ModuleSymbolTable.cpp#L207
[4]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L1664-L1668

On Wed, 8 Sept 2021 at 20:03, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:>
> For the specific problem hit below, it feels like another available
approach would be to change GlobalIndirectSymbol to behave correctly for both
GlobalAlias and GlobalIFunc, without changing the class hierarchy, by reducing
its scope and deferring more to its derived classes (e.g., change
getBaseObject() to do the right thing).
>
> Can you comment further on the tradeoffs vs. the refactoring you’re
proposing? (I see your argument that globalifunc shares some properties
globalobject, but it’s not obvious to me that it’s more similar to globalobject
than it is to globalalias, or that in aggregate code dealing with these classes
will be cleaner after moving globalifunc over to the globalobject bucket.)
>
> > On 2020-09-07, at 13:06, Itay Bookstein via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> >
> > I was working on https://reviews.llvm.org/D81911 to try and fix
> > https://bugs.llvm.org/show_bug.cgi?id=46340 . Through the review
> > process we happened upon a design limitation or perhaps a potential
> > mis-modelling of GlobalIFunc in the IR object hierarchy, which leads
> > to some problems in LTO flows.
> >
> > To summarize, as it currently stands (and in the hopes of faithfully
> > representing the conclusions of that discussion):
> > * Calling getBaseObject() on a GlobalAlias whose aliasee is a
> > GlobalIFunc currently returns null.
> > * Calling getBaseObject() on a GlobalIFunc returns its resolver
function.
> > * This causes computeAliasSummary in ModuleSummaryAnalysis to crash on
> > a null dereference for an alias-to-ifunc situation.
> > * A GlobalIFunc and its resolver are *not* interchangeable: at the
> > interface level, they have different signatures (conceptually, the
> > IFunc has the same signature of the function pointer that the resolver
> > potentially returns, not of the resolver itself).
> > * It makes sense for the IFunc to be its own base object (which is
> > GlobalObject-like-behavior), but type-hierarchy-wise it can't.
This is
> > because GlobalIFunc derives from GlobalIndirectSymbol which derives
> > directly from GlobalValue, and therefore it is not a GlobalObject.
> >
> > Would it make sense for GlobalIFunc to derive from GlobalObject
> > instead of GlobalIndirectSymbol? If so, GlobalIndirectSymbol would
> > lose its purpose somewhat, and could be merged into GlobalAlias. It
> > would, however, require updating a considerable amount of code.
> >
> > ~Itay
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 

Itay Bookstein

Software Engineer

NEXTSILICON

-- 
This e-mail message and any attachments thereto are intended only for the 
person or entity to which it is addressed and may contain confidential 
and/or privileged material. Any retransmission, dissemination, copying or 
other use of, or taking of any action in reliance upon this information is 
prohibited. If you are not the intended addressee, please contact the 
sender immediately and delete the materials and information from your 
device and system and confirm the deletion by reply e-mail.

Fangrui Song via llvm-dev

2021-Sep-14 17:26 UTC

head link

[llvm-dev] [IR] Modelling of GlobalIFunc

On 2021-09-11, Itay Bookstein via llvm-dev wrote:>That really depends on what the correct behavior is for
>getBaseObject(). As far as I can tell, this method was originally
>added to pierce through GlobalAliases (including various
>ConstantExpr-s) and get to the "subject" *GlobalObject* (which is
its
>return type).
>
>When looking at getBaseObject() through these
>GlobalAlias-colored-glasses, I think that the following invariant
>should follow:
>* getBaseObject() should be idempotent (at least for the cases that it
>does not return a nullptr): GV->getBaseObject()->getBaseObject()
=>GV->getBaseObject().
>Note that a GlobalAlias may have a different type than its base object
>[4], so my previous arguments about a same-type-invariant are
>incorrect.
>
>Now, let us consider the case of a GlobalAlias where the aliasee is a
>GlobalIFunc. This happens in glibc for example.
>What should GA->getBaseObject() return in this case? I think there are
>three options here:
>1. nullptr; this sort-of-misses the original point of getBaseObject(),
>since there *is* an aliasee, you don't even need to pierce through
>ConstantExpr-s to get it.
>2. The GlobalIFunc itself; this is only possible with either:
>2. a. Making GlobalIFunc inherit from GlobalObject [1].
>2. b. Changing the return type of getBaseObject() to be GlobalValue
>(which also seems to be missing the original point of
>getBaseObject()).
>3. The GlobalIFunc's resolver; this is similar to the behavior of
>getRootObject() in my PR [1].
>
>The current situation on top of master is that GA->getBaseObject()
>would return nullptr, but GI->getBaseObject() would return the
>resolver, which makes it non-idempotent.
>To resolve that, I think that either 2.a. or 3. make sense here.
I'd just avoid llvm::GlobalIndirectSymbol::getBaseObject for GlobalIFunc.
getBaseObject can be moved to GlobalAlias:: to avoid GlobalIndirectSymbol
confusion.
It seems that most users only call getBaseObject with a GlobalAlias, so the
required changes are
few.

For GlobalIFunc, just use getResolver().
>Some users [2] consider the result of GlobalAlias::getBaseObject() to
>be the aliasee [2], which option 3 doesn't uphold.
>Other users [3] consider the result of getBaseObject() to be some
>"root" / "key" / "leader" for a group of
GlobalValues that need to be
>handled together, which option 2 doesn't uphold. This is the reason
>for the getBaseObject/getRootObject split in my PR, which demonstrates
>2.a.
>
>There's also the replication of some of this type hierarchy and
>semantics in {GlobalValue,Alias,Function,GlobalVar}Summary::getBaseObject(),
>which I haven't yet considered. Note:
>* IFunc is missing from that list
>* GlobalValueSummary::getBaseObject() returns a GlobalValueSummary,
>not a GlobalObjectSummary (which doesn't exist).
With the changes above, there is probably not much in GlobalIndirectSymbol which
can be shared with
GlobalAlias and GlobalIFunc, but that seems ok to me.
>Thoughts?
>
>[1] https://reviews.llvm.org/D108872
>[2]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L4037
>https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp#L640
>https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L1675
>https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/LTO/LTO.cpp#L736
>[3]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Transforms/Utils/SplitModule.cpp#L130
>https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Transforms/Utils/SplitModule.cpp#L229
>https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/Object/ModuleSymbolTable.cpp#L207
>[4]
https://github.com/llvm/llvm-project/blob/6aacc69338787bfa1ad814928459e3cb94522298/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L1664-L1668
>
>
>On Wed, 8 Sept 2021 at 20:03, Duncan P. N. Exon Smith
><dexonsmith at apple.com> wrote:
>>
>> For the specific problem hit below, it feels like another available
approach would be to change GlobalIndirectSymbol to behave correctly for both
GlobalAlias and GlobalIFunc, without changing the class hierarchy, by reducing
its scope and deferring more to its derived classes (e.g., change
getBaseObject() to do the right thing).
>>
>> Can you comment further on the tradeoffs vs. the refactoring you’re
proposing? (I see your argument that globalifunc shares some properties
globalobject, but it’s not obvious to me that it’s more similar to globalobject
than it is to globalalias, or that in aggregate code dealing with these classes
will be cleaner after moving globalifunc over to the globalobject bucket.)
>>
>> > On 2020-09-07, at 13:06, Itay Bookstein via llvm-dev <llvm-dev
at lists.llvm.org> wrote:
>> >
>> > I was working on https://reviews.llvm.org/D81911 to try and fix
>> > https://bugs.llvm.org/show_bug.cgi?id=46340 . Through the review
>> > process we happened upon a design limitation or perhaps a
potential
>> > mis-modelling of GlobalIFunc in the IR object hierarchy, which
leads
>> > to some problems in LTO flows.
>> >
>> > To summarize, as it currently stands (and in the hopes of
faithfully
>> > representing the conclusions of that discussion):
>> > * Calling getBaseObject() on a GlobalAlias whose aliasee is a
>> > GlobalIFunc currently returns null.
>> > * Calling getBaseObject() on a GlobalIFunc returns its resolver
function.
>> > * This causes computeAliasSummary in ModuleSummaryAnalysis to
crash on
>> > a null dereference for an alias-to-ifunc situation.
>> > * A GlobalIFunc and its resolver are *not* interchangeable: at the
>> > interface level, they have different signatures (conceptually, the
>> > IFunc has the same signature of the function pointer that the
resolver
>> > potentially returns, not of the resolver itself).
>> > * It makes sense for the IFunc to be its own base object (which is
>> > GlobalObject-like-behavior), but type-hierarchy-wise it can't.
This is
>> > because GlobalIFunc derives from GlobalIndirectSymbol which
derives
>> > directly from GlobalValue, and therefore it is not a GlobalObject.
>> >
>> > Would it make sense for GlobalIFunc to derive from GlobalObject
>> > instead of GlobalIndirectSymbol? If so, GlobalIndirectSymbol would
>> > lose its purpose somewhat, and could be merged into GlobalAlias.
It
>> > would, however, require updating a considerable amount of code.
>> >
>> > ~Itay
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
>-- 
>
>
>
>
>Itay Bookstein
>
>Software Engineer
>
>
>NEXTSILICON
>
>-- 
>This e-mail message and any attachments thereto are intended only for the
>person or entity to which it is addressed and may contain confidential
>and/or privileged material. Any retransmission, dissemination, copying or
>other use of, or taking of any action in reliance upon this information is
>prohibited. If you are not the intended addressee, please contact the
>sender immediately and delete the materials and information from your
>device and system and confirm the deletion by reply e-mail.
>_______________________________________________
>LLVM Developers mailing list
>llvm-dev at lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

llvm dev - Sep 2021 - [IR] Modelling of GlobalIFunc

[llvm-dev] [IR] Modelling of GlobalIFunc

[llvm-dev] [IR] Modelling of GlobalIFunc