This is an archive of the discontinued LLVM Phabricator instance.

[llvm-c] Remove bindings for creating legacy passes
ClosedPublic

Authored by nikic on Feb 28 2023, 7:59 AM.

Details

Summary

Legacy passes are only supported for codegen, and I don't believe it's possible to write backends using the C API, so we should drop all of those. Reduces the number of places that need to be modified when removing legacy passes.

Diff Detail

Event Timeline

nikic created this revision.Feb 28 2023, 7:59 AM
Herald added a project: Restricted Project. · View Herald Transcript
nikic requested review of this revision.Feb 28 2023, 7:59 AM
aeubanks added inline comments.Feb 28 2023, 10:56 AM
llvm/lib/Transforms/IPO/IPO.cpp
33

all of these initialize functions should also be deleted (everything in Initialization.h)

nikic added inline comments.Feb 28 2023, 1:01 PM
llvm/lib/Transforms/IPO/IPO.cpp
33

Isn't it necessary to still call initialization functions for passes used in the codegen pipeline?

aeubanks added inline comments.Feb 28 2023, 1:11 PM
llvm/lib/Transforms/IPO/IPO.cpp
33

LLVMInitializeIPO (and others) are part of the C API, which as mentioned in the description can't be used for the codegen pipeline. We'll still keep around llvm::initializeIPO above. Actually we should just delete LLVMPassRegistryRef altogether

nikic added inline comments.Feb 28 2023, 1:25 PM
llvm/lib/Transforms/IPO/IPO.cpp
33

I'm referring to users of the codegen pipeline here. Is it not necessary to initialize codegen passes before using the codegen pipeline? Or is it only necessary if you want to refer to passes by name?

nikic added inline comments.Feb 28 2023, 2:12 PM
llvm/lib/Transforms/IPO/IPO.cpp
33

Ah, I guess this is not necessary because passes call their own initialization function in their constructor (or at least that seems to be the convention), so they should get automatically initialized if created via the C++ API, rather than by name.

nikic added inline comments.
llvm/lib/Transforms/IPO/IPO.cpp
33

I've submitted a separate patch for this change (D145043), because I think it will be much higher impact than this one.

aeubanks accepted this revision.Mar 1 2023, 9:19 AM
aeubanks added inline comments.
llvm/lib/Transforms/IPO/IPO.cpp
33

Ah I missed that there are C APIs to run the codegen pipeline.

I believe initialization is only necessary for functionality around names (e.g. for debugging), but yeah pass constructors typically initialize themselves.

This revision is now accepted and ready to land.Mar 1 2023, 9:19 AM
This revision was landed with ongoing or failed builds.Mar 2 2023, 12:54 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2023, 12:54 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
yamt added a subscriber: yamt.Jun 20 2023, 2:55 AM

at least some of these stuff is used by wasm-micro-runtime. do you have a suggestion about how to adapt it?
https://github.com/bytecodealliance/wasm-micro-runtime/blob/72fc872afe9a51228b2a32bc7b08475b176f187f/core/iwasm/compilation/aot_compiler.c#L2666-L2670

nikic added a comment.Jun 20 2023, 3:04 AM

at least some of these stuff is used by wasm-micro-runtime. do you have a suggestion about how to adapt it?
https://github.com/bytecodealliance/wasm-micro-runtime/blob/72fc872afe9a51228b2a32bc7b08475b176f187f/core/iwasm/compilation/aot_compiler.c#L2666-L2670

You can use LLVMRunPasses with lower-atomic,lower-switch or so.

llvm/include/llvm-c/Transforms/Scalar.h