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

Fri Sep 24 16:04:16 PDT 2021

pcc requested changes to this revision.
pcc added a comment.
This revision now requires changes to proceed.

In D108741#3011080 <https://reviews.llvm.org/D108741#3011080>, @kubamracek wrote:

>> 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.
>
> As far as I can tell, VFE was added via https://reviews.llvm.org/D63932 and it has since always used type metadata and the !type annotations on vtables. Are you asking for a reversal on how VFE is done at the IR level, including how it's done for C++ in Clang? I'd love to see more thought on that (and what exactly do you think the design should aim to do better), but it seems to me that that's out of scope for this particular small tweak that I'm pursuing.

Sorry, I think I managed to confuse myself when I was trying to figure out what VFE actually relies on. It seems that VFE doesn't rely on what I thought it did. Although VFE relies on type metadata, it only relies on it to determine the locations of the address points, and not the locations of the VFE-eligible function slots.

Historically we've only ever used VFE with C++, which is fine with any virtual function slot being eligible for VFE. As such, we implemented it to make that assumption. What we should really be doing is have a way of marking slots as eligible, and we never got a chance to design that because it wasn't necessary. Now that you're proposing to use VFE with a language which presumably does have some non-VFE-eligible virtual function slots, I think we need to design such a mechanism.

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.

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

https://reviews.llvm.org/D108741