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

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 20 15:42:34 PDT 2021


pcc added a comment.



In D108741#3005803 <https://reviews.llvm.org/D108741#3005803>, @fhahn wrote:

> In D108741#2998705 <https://reviews.llvm.org/D108741#2998705>, @tejohnson wrote:
>
>> In D108741#2997844 <https://reviews.llvm.org/D108741#2997844>, @fhahn wrote:
>>
>>>> As part of codesize optimization work for Swift, we'd like to add Virtual Function Elimination to Swift, very similarly to how GlobalDCE supports C++ VFE.
>>>
>>> I think it would be good to drop this part from the description, as the change is not really related to swift; it just changes the code to ignore entries at slots without `!type`.
>>>
>>> It seems sensible to me to ignore entries without corresponding `!type` metadata  and this should be in line with the specification in langref. @pcc, @tejohnson Are there any issues you could think of?
>>
>> Is this specified in the langref? I looked through the type metadata documentation but don't see it mentioned there that all vfuncs will have type metadata. Looks like that was added a bit later than the original type metadata, in D47567 <https://reviews.llvm.org/D47567>. I think what is specified in langref is that it has type metadata for the address point. Probably the documentation needs updating. But from what I can see, it looks like this should always be emitted now. @pcc ?
>
> Thanks for checking! I think the update to the langref in the latest version should spell out things clearly.

It isn't great that VFE apparently relies on the type metadata that we added to support member function pointer CFI in C++. I was unaware that this was the case, and it shouldn't be necessary for frontends that don't have something like C++'s peculiar member function pointer ABI (I assume that Swift doesn't?) It would be better if we designed an independent way of marking these entries as subject to VFE.


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

https://reviews.llvm.org/D108741



More information about the llvm-commits mailing list