[PATCH] D108741: [GlobalDCE] Handle non-vfunc entries in vtables during VFE

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

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>

We shouldn't be hesitant to extend the IR in order to model things correctly. And given that we're introducing all these new Constant types it may make sense to refactor to make adding new Constants easier (doesn't need to happen now though). In the end I think more complexity would result from an incorrect modelling (e.g. frontends would have to make up unnecessary type names to cause virtual function slots to be subject to VFE, and what if a frontend wants VFE for globals, or exempt its address points from VFE -- would we need another opt-in for those?).


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

https://reviews.llvm.org/D108741