-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement LLVM IR Virtual Function Elimination for Swift classes. #39128
Conversation
lib/IRGen/GenMeta.cpp
Outdated
var->addTypeMetadata(offset, typeIdForMethod(*this, method)); | ||
} | ||
|
||
switch (getOptions().VCallVisibility) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not deriving the VCall visibility from the method visibility?
Setting the visibility for all calls sounds dangerous, e.g. if a public method gets a "File" visibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, it is dangerous -- incorrectly set visibility will cause miscompiles. I've added this to be used in tests, where I need to make sure the method isn't going to eliminated by the SIL optimizer, so it's public, but I want the LLVM IR optimizer to eliminate it, therefore the vcall visibility set to "file".
Open to any better suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can call a private/internal method from a function, annotated with @_optimize(none)
. That will keep the virtual call alive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the PR to infer the vtable visibility from the class decl's access scope visibility.
lib/IRGen/GenMeta.cpp
Outdated
@@ -334,6 +334,33 @@ void IRGenModule::emitNonoverriddenMethodDescriptor(const SILVTable *VTable, | |||
getAddrOfLLVMVariable(entity, init, DebugTypeInfo()); | |||
} | |||
|
|||
void IRGenModule::addVTableTypeMetadata(llvm::GlobalVariable *var, | |||
SmallVector<std::pair<Size, SILDeclRef>, 8> vtableEntries) { | |||
if (!getOptions().VirtualFunctionElimination) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already checked at the call site
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
It would be good if @aschwaighofer takes a look, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@swift-ci please test |
Build failed |
@swift-ci please test |
Build failed |
@swift-ci please test |
@swift-ci please test |
- Virtual calls are done via a @llvm.type.checked.load instrinsic call with a type identifier - Type identifier of a vfunc is the base method's mangling - Type descriptors and class metadata get !type markers that list offsets and type identifiers of all vfuncs - The -enable-llvm-vfe frontend flag enables VFE - Two added tests verify the behavior on IR and by executing a program
@swift-ci please test |
Depends on changes in LLVM (some unmerged yet): https://reviews.llvm.org/D107645, https://reviews.llvm.org/D108741, https://reviews.llvm.org/D109114, https://reviews.llvm.org/D109169.