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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
