[PATCH] D108741: [IR][GlobalDCE] Add ability to mark vtable methods as eligible for VFE and avoid eliminating non-eligible vfunc in VFE in GlobalDCE

Thu Sep 30 09:06:01 PDT 2021

fhahn added a comment.

In D108741#3025712 <https://reviews.llvm.org/D108741#3025712>, @pcc wrote:

> In D108741#3023267 <https://reviews.llvm.org/D108741#3023267>, @fhahn wrote:
>
>> In D108741#3021933 <https://reviews.llvm.org/D108741#3021933>, @pcc wrote:
>>
>>> In D108741#3021793 <https://reviews.llvm.org/D108741#3021793>, @kubamracek wrote:
>>>
>>>> @pcc, could I ask for some details about your last comment? I'm specifically wondering whether you're against proceeding with the change from this review.
>>>
>>> Sorry, but I think I'm against it because the mechanism that you've chosen seems like an abuse of the !type metadata, since the metadata is meant to be used as a reference point for accesses relative to it, and not as a way of marking up specific fields by itself. As a more practical concern, it has no way of representing the situation where the pointer at the address point is not eligible for VFE.
>>>
>>> Here's one alternative proposal: can we make it so that any vtable slots that are eligible for VFE are represented using a new type of Constant (similar to dso_local_equivalent)? This also has the advantage that frontends could also use it for global variables as well as functions.
>>
>> A potentially more lightweight solution may be providing a way to opt-in to the stricter interpretation. That way, we ensure no existing users are disturbed while not having to add more invasive changes. Do you think the additional complexity a new type of Constant carries its weight if it is only used to support the swift VFE case?
>
> Adding a new Constant type isn't very difficult, see e.g. D108478 <https://reviews.llvm.org/D108478>

Ah right, that indeed looks much less work than I thought, thanks!

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

https://reviews.llvm.org/D108741