Skip to content

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

Merged
merged 1 commit into from
Sep 21, 2021

Conversation

kubamracek
Copy link
Contributor

  • 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

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 2e1451a568078e7e426fdd8a2fe3c3a89dc34417

@slavapestov
Copy link
Contributor

What optimization opportunities does this open up that are not handled by devirtualization at the SIL level?

@kubamracek
Copy link
Contributor Author

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.

Copy link
Contributor

@eeckstein eeckstein left a 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);
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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
@kubamracek
Copy link
Contributor Author

@swift-ci please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants