[libcxx-commits] [PATCH] D72959: Relative VTables ABI on Fuchsia

[John McCall via Phabricator via libcxx-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGVTables.cpp:623
+    llvm::Constant *C, llvm::GlobalVariable *VTable, unsigned vtableIdx,
+    unsigned lastAddrPoint) const {
+  // No need to get the offset of a nullptr.
----------------
leonardchan wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > There's already an `addRelativeOffset` on `ConstantArrayBuilder`; is that insufficient for some reason?  I think that, if v-table building were refactored so that the places that build components also add them to the v-table, we'd end up with a lot more flexibility for the ABIs.  We needed a similar sort of change for pointer authentication, which we haven't upstreamed to LLVM yet, but which you can see here:
> > > 
> > > https://github.com/apple/llvm-project/blob/apple/master/clang/lib/CodeGen/CGVTables.cpp
> > > 
> > > 
> > I actually did not know about this method, but it does seem to boil down to the same arithmetic used here. Will update to see if I can use the existing builders instead.
> Ah, so it seems `addRelativeOffset` operates differently than I thought. Initially I thought it just took the offset relative to the start of the array being built, but it actually seems to be relative to the component that would be added to the builder. This is slightly different from the current implementation where the offset is instead taken relative to the address point of the current vtable.
> 
> We could technically still achieve the desired effect of offsets in the vtable if we were to use `addTaggedRelativeOffset` and subtract a constantaly increasing offset as more virtual function components are added, although I'm not sure how much more benefit that would offer.
> 
> An alternative approach could also be to just use `addRelativeOffset` for offsets relative to the component slot and update instances we access the vtable to adjust for this new arithmetic. I think this would just transform instances of `llvm.load.relative(%vtable, %func_offset)` to `llvm.load.relative(%vtable + %func_offset , 0)`, which seems differerent from how it was intended to be used, but technically might work.
Interesting.  Yeah, it should be relatively straightforward to add a method that adds a pointer subtraction starting from a different base, and there's already a function for getting a constant that represents the address of the current position in the struct.

I take it you've taught LLVM that it can emit that subtraction with a normal relative-offset relocation by using an addend?  That must've been exciting.  Or, yeah, we can add in that addend in the frontend.

I think I see the expected benefit: it should generally save an instruction from the call sequence, at least on targets like x86 that lack pre-incrementing addressing modes.  It does mean that the v-table entries can't be independently described — e.g. there's really no way you could write a C++ type that behaves like a v-table entry — but that's not too much of a loss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72959