[PATCH] D77592: [NFC][CodeGen] Add enum for selecting the layout of components in the vtable

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked an inline comment as done.
leonardchan added a comment.

In D77592#1985740 <https://reviews.llvm.org/D77592#1985740>, @rjmccall wrote:

> I'm not sure if the AST-level v-table layout abstraction really cares about these differences.  I don't think it vends byte offsets into the v-table, just slot indices (i.e. word offsets).


The primary reason for why I added it in `AST` is because down the line, to support the changes to the RTTI component, I'll need to edit `VCallAndVBaseOffsetBuilder::getCurrentOffsetOffset()`:

  CharUnits VCallAndVBaseOffsetBuilder::getCurrentOffsetOffset() const {
    // OffsetIndex is the index of this vcall or vbase offset, relative to the
    // vtable address point. (We subtract 3 to account for the information just
    // above the address point, the RTTI info, the offset to top, and the
    // vcall offset itself).
    int64_t OffsetIndex = -(int64_t)(3 + Components.size());
  
    CharUnits PointerWidth =
      Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
    CharUnits OffsetOffset = PointerWidth * OffsetIndex;
    return OffsetOffset;
  }

Right now, it assumes that the offset is 3 pointer widths below the vtable address point, but since we want to make the RTTI component a 32-bit int, we'll need to add an extra 4 `CharUnits` to the result. The enum would need to be exposed on the AST level so we can check it here. If we move the enum into `ItaniumCXXABI` or somewhere under codegen, then we will instead need to check every instance vcall or vbase offsets are used in codegen. I tried playing around with this idea, but I gave up since there seemed to be too many instances I didn't catch since my tests kept failing. I figured this was the easiest way.

This is just my experience trying to move things around, but it could be there's a better place to put this that I'm not seeing.



================
Comment at: clang/include/clang/AST/VTableBuilder.h:380
+    /// other structs/functions.
+    Relative,
+  };
----------------
rjmccall wrote:
> The only "struct" pointed to by the v-table is the `type_info` object.  How are you planning to handle that?  The standard ABI makes the `type_info` a vague-linkage symbol in most cases, so you won't be able to have a direct relative reference to it.  If you adopt the Apple ARM64 modification for `type_info` equality, you can rely on the `type_info` being defined within the same linkage unit unless the v-table is being emitted as an optimization for a type defined with a key function in a different linkage unit.  You could handle that by making this an "inidirectable" relative reference, where it's either a direct relative reference or a relative reference to a GOT entry, as specified by the low bit in the reference.  But you need *some* solution for this.
The plan I have is to add the "indirectable" relative reference you mention. In IR, we emit a `dso_local` global that points to the potentially external `type_info` object, and the relative reference is taken on this `dso_local` symbol. This IR seems to get lowered to a relative reference to a GOT entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77592