-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Implement LLVM IR Witness Method Elimination for Swift witness tables. #39287
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
Conversation
kubamracek
commented
Sep 13, 2021
- Witness method calls are done via @llvm.type.checked.load instrinsic call with a type identifier
- Type id of a witness method is the requirement's mangled name
- Witness tables get !type markers that list offsets and type ids of all methods in the wtable
- Added -enable-llvm-wme to enable Witness Method Elimination
- Added IR test and execution test
@swift-ci please test |
Build failed |
What optimization opportunities does this open up that are not handled by devirtualization at the SIL level? |
Cross-module optimizations at LTO time. Today, any library/framework that's built separately from client code cannot eliminate any methods from public types, because of missing information about what is actually used or not. |
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.
@aschwaighofer should take a look, too.
|
||
llvm::Value *checkedLoad = | ||
IGF.Builder.CreateCall(checkedLoadIntrinsic, args); | ||
return IGF.Builder.CreateExtractValue(checkedLoad, 0); |
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.
Do we loose the information that the load is invariant? This could lead to code size losses as compared to the regular mode because redundant loads are not 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.
I think you're right, we're losing the "invariant" marker here. It's not even a LoadInst anymore, so we can't just add it. But I think it should be possible to improve how LLVM lowers the @llvm.type.checked.load
call into an actual load, and pass an invariant marker to it that way. But that needs some LLVM support first -- would you be okay if I mentioned it here as a TODO/FIXME comment and left it as a separate improvement to follow up?
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.
Sounds good to me.
- Witness method calls are done via @llvm.type.checked.load instrinsic call with a type identifier - Type id of a witness method is the requirement's mangled name - Witness tables get !type markers that list offsets and type ids of all methods in the wtable - Added -enable-llvm-wme to enable Witness Method Elimination - Added IR test and execution test
@swift-ci please test |