leonardchan added a comment. Herald added a subscriber: jdoerfert. Hi @pcc , I'm working on revisiting this to see if this could help when building Fuchsia (D58321 <https://reviews.llvm.org/D58321>) and had a few questions I left inline.
================ Comment at: lib/CodeGen/CGVTables.cpp:532 + for (auto &Comp : VTLayout.vtable_components()) { + if (Comp.isFunctionPointerKind()) + Types.push_back(CGM.Int32Ty); ---------------- It seems that the condition for treating a vtable component as an i32 vs i8* is slightly different here than in `SetVTableInitializer`. `SetVTableInitializer` checks ``` Component.isFunctionPointerKind() && (I == 0 || !Components[I - 1].isFunctionPointerKind()) ``` where `GetVTableType` just checks `Component.isFunctionPointerKind()`. Should this also check if the first or last component is a function pointer, and does this check in `SetVTableInitializer` have any importance with the FIXME below it? ================ Comment at: test/CodeGenCXX/vtable-relative-abi.cpp:5 + +// CHECK-ITANIUM: @_ZTV1S = hidden unnamed_addr constant { i8*, i8*, i32, i32 } { i8* null, i8* bitcast ({ i8*, i8* }* @_ZTI1S to i8*), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @_ZN1S2f1Ev to i64), i64 ptrtoint (i32* getelementptr inbounds ({ i8*, i8*, i32, i32 }, { i8*, i8*, i32, i32 }* @_ZTV1S, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @_ZN1S2f2Ev to i64), i64 ptrtoint (i32* getelementptr inbounds ({ i8*, i8*, i32, i32 }, { i8*, i8*, i32, i32 }* @_ZTV1S, i32 0, i32 2) to i64)) to i32) }, align 8 +// CHECK-MS: @0 = private unnamed_addr constant { i8*, i32, i32 } { i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4S@@6B@" to i8*), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @"\01?f1@S@@UEAAXXZ" to i64), i64 ptrtoint (i32* getelementptr inbounds ({ i8*, i32, i32 }, { i8*, i32, i32 }* @0, i32 0, i32 1) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (void (%struct.S*)* @"\01?f2@S@@UEAAXXZ" to i64), i64 ptrtoint (i32* getelementptr inbounds ({ i8*, i32, i32 }, { i8*, i32, i32 }* @0, i32 0, i32 1) to i64)) to i32) }, comdat($"\01??_7S@@6B@") ---------------- Should the second `getelementptr` access be `i32 0, i32 3` instead since it’s for the 4th element in the struct? If so, I think this is due to some referencing problem with the `maybeMakeRelative` lambda. I ran into this issue when I moved `llvm::Type *VTableTy = VTable->getValueType();` into the following if block that initializes `AddrPointInt`. This also seems to occur for other test classes that contain more than one virtual function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20749/new/ https://reviews.llvm.org/D20749 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits