[PATCH] D72959: Relative VTables ABI on Fuchsia

leonardchan abandoned this revision. leonardchan marked an inline comment as done. leonardchan added a comment. Herald added a subscriber: bd1976llvm. This was landed in various other smaller changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D729

[PATCH] D72959: Relative VTables ABI on Fuchsia

ldionne added a comment. Herald added subscribers: abrachet, jdoerfert, pengfei, MaskRay. Herald added a project: All. [Github PR transition cleanup] I think this work has landed as multiple separate patches, right? If so, can we please close this to clear up the review queue? Repository: rG

[PATCH] D72959: Relative VTables ABI on Fuchsia

rjmccall added a comment. Otherwise LGTM. Comment at: clang/lib/CodeGen/CGVTables.cpp:633 + llvm::Module &module = CGM.getModule(); + llvm::SmallString<16> componentName(globalVal->getName()); + `componentName` seems to no longer be used meaningfully. in this

[PATCH] D72959: Relative VTables ABI on Fuchsia

leonardchan added inline comments. Comment at: clang/lib/CodeGen/CGVTables.cpp:646 + +// Take offset from stub to the vtable component. +llvm::Function *HiddenFunc = M.getFunction(StubName); rjmccall wrote: > That's not what this block is doing. > > I th

[PATCH] D72959: Relative VTables ABI on Fuchsia

rjmccall added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:357 +"allow use of clang's relative C++ ABI " +"for classes with vtables on targets that support this") "Use an ABI-incompatible v-table layout that uses relat

[PATCH] D72959: Relative VTables ABI on Fuchsia

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: > leona

[PATCH] D72959: Relative VTables ABI on Fuchsia

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 nul

[PATCH] D72959: Relative VTables ABI on Fuchsia

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 nul

[PATCH] D72959: Relative VTables ABI on Fuchsia

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. There's already an `addRel

[PATCH] D72959: Relative VTables ABI on Fuchsia

phosek added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:353 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0") +LANGOPT(FuchsiaRelativeCXXABIVTables, 1, 0, +"Whether to use clang's relative C++ ABI " ---

[PATCH] D72959: Relative VTables ABI on Fuchsia

mcgrathr added inline comments. Comment at: libcxxabi/src/private_typeinfo.cpp:617 // Get (dynamic_ptr, dynamic_type) from static_ptr +#ifndef __Fuchsia__ void **vtable = *static_cast(static_ptr); ldionne wrote: > Please introduce a macro that generical

[PATCH] D72959: Relative VTables ABI on Fuchsia

ldionne requested changes to this revision. ldionne added inline comments. Comment at: libcxxabi/src/private_typeinfo.cpp:617 // Get (dynamic_ptr, dynamic_type) from static_ptr +#ifndef __Fuchsia__ void **vtable = *static_cast(static_ptr); Please introd

[PATCH] D72959: Relative VTables ABI on Fuchsia

rjmccall added a comment. Okay. In that case, I would ask that if you're considering going through the work of adding relocations, please consider adding both scaled and unscaled relative-offset-to-equivalent-symbol. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D72959: Relative VTables ABI on Fuchsia

peter.smith added a comment. In D72959#1835368 , @rjmccall wrote: > There's two independently-useful things here for the relocation, right? > First, it's useful to be able to express a relocation to a symbol that has > the semantics of a particular func

[PATCH] D72959: Relative VTables ABI on Fuchsia

rjmccall added a comment. There's two independently-useful things here for the relocation, right? First, it's useful to be able to express a relocation to a symbol that has the semantics of a particular function but doesn't necessarily have its address, and that concept could be used across ma

[PATCH] D72959: Relative VTables ABI on Fuchsia

peter.smith added a comment. In D72959#1832011 , @pcc wrote: > > On Aarch64, right now only CALL26 and JUMP26 instructions generate PLT > > relocations, so we manifest them with stubs that are just jumps to the > > original function. > > I think it would

[PATCH] D72959: Relative VTables ABI on Fuchsia

pcc added a subscriber: peter.smith. pcc added a comment. > On Aarch64, right now only CALL26 and JUMP26 instructions generate PLT > relocations, so we manifest them with stubs that are just jumps to the > original function. I think it would be worth considering defining a new relocation type f