leonardchan marked an inline comment as done.
leonardchan 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:
> 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.
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
