This is an archive of the discontinued LLVM Phabricator instance.

[llvm-c] Remove PassRegistry and initialization APIs
ClosedPublic

Authored by nikic on Mar 1 2023, 1:18 AM.

Details

Summary

Remove C APIs for interacting with PassRegistry and pass initialization. As pointed out in D144970, these are legacy PM concepts that are no longer relevant for the new pass manager. Remaining uses of the legacy passes in the codegen pipeline will be initialized automatically.

Depends on D144970.

Diff Detail

Event Timeline

nikic created this revision.Mar 1 2023, 1:18 AM
Herald added a project: Restricted Project. · View Herald Transcript
nikic requested review of this revision.Mar 1 2023, 1:18 AM
nikic updated this revision to Diff 501416.Mar 1 2023, 1:21 AM

Add release notes.

aeubanks accepted this revision.Mar 1 2023, 9:20 AM
This revision is now accepted and ready to land.Mar 1 2023, 9:20 AM
MaskRay accepted this revision.Mar 2 2023, 1:32 PM
MaskRay added a comment.EditedMar 2 2023, 1:37 PM

It'd be great if you can wait a bit on landing this.

LLVMGetGlobalPassRegistry is called by some projects such as

and we want to determine whether simply commenting out the functions locally can work.

nikic added a comment.Mar 2 2023, 1:40 PM

@MaskRay Sure, I'm also waiting on https://github.com/rust-lang/rust/pull/108599 to land first.

Just to put it out there, an alternative to this patch is to retain all these functions and just make them no-ops. Maybe that's the better option in terms of reducing C API breakage?

@MaskRay Sure, I'm also waiting on https://github.com/rust-lang/rust/pull/108599 to land first.

Thank you! Also, we managed to integrate D144970 but it may be useful to wait for the change to stabilize...

Just to put it out there, an alternative to this patch is to retain all these functions and just make them no-ops. Maybe that's the better option in terms of reducing C API breakage?

Sounds good!

IMO it's better to delete API surfaces (not deleting one off functions since that does cause unnecessary churn, but e.g. everything that revolves around LLVMPassRegistryRef) that don't do anything, else it can be confusing for people to figure out how to use the C API when a good portion of them are no-ops

but yeah short term delaying this patch is fine

nikic added a comment.Mar 30 2023, 1:41 AM

Just wanted to check back here whether we're ready to land this patch now or should wait longer?

I think we should be ok to go forward with this. can you put in the description that users can just delete calls to these functions

This revision was landed with ongoing or failed builds.Apr 14 2023, 3:13 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 3:13 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
llvm/include/llvm-c/Core.h