Skip to content
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

[CodeGen] Port ShadowStackGCLowering to new pass manager #75324

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

paperchalice
Copy link
Contributor

IIUC the new pass system was designed with parallelism. This pass needs to add some global variables into the current module, this is not allowed by
WritingAnLLVMPass, so convert it to module pass, see FIXME in GetFrameMap.

Therefore, this will trigger assertion in CodeGenPassBuilder:

// The codegen IR pipeline are mostly function passes with the exceptions of
// a few loop and module passes. `AddingFunctionPasses` make sures that
// we could only add module passes at the beginning of the pipeline. Once
// we begin adding function passes, we could no longer add module passes.
// This special-casing introduces less adaptor passes. If we have the need
// of adding module passes after function passes, we could change the
// implementation to accommodate that.
std::optional<bool> AddingFunctionPasses;
Will fix it in future.

@paperchalice
Copy link
Contributor Author

Ping @arsenm

@@ -170,7 +212,7 @@ Type *ShadowStackGCLowering::GetConcreteStackEntryType(Function &F) {

/// doInitialization - If this module uses the GC intrinsics, find them now. If
/// not, exit fast.
bool ShadowStackGCLowering::doInitialization(Module &M) {
bool ShadowStackGCLoweringImpl::doInitialization(Module &M) {
bool Active = false;
for (Function &F : M) {
if (F.hasGC() && F.getGC() == std::string("shadow-stack")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing issue but this doesn't need the std::string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@paperchalice paperchalice force-pushed the NPM/CodeGen/ss branch 2 times, most recently from 6ba3523 to 3892562 Compare December 19, 2023 07:20
@paperchalice paperchalice merged commit 1c43e64 into llvm:main Jan 6, 2024
4 checks passed
@paperchalice paperchalice deleted the NPM/CodeGen/ss branch January 6, 2024 01:30
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
IIUC the new pass system was designed with parallelism. This pass needs
to add some global variables into the current module, this is not
allowed by

[WritingAnLLVMPass](https://llvm.org/docs/WritingAnLLVMPass.html#the-functionpass-class),
so convert it to module pass, see FIXME in `GetFrameMap`.

Therefore, this will trigger assertion in `CodeGenPassBuilder`:
https://github.com/llvm/llvm-project/blob/effd47ed45e3badd756103346a7c3b9e1e939e5e/llvm/include/llvm/CodeGen/CodeGenPassBuilder.h#L200-L207
Will fix it in future.
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.

None yet

2 participants