[CAPI] disambiguate mlirRegisterConversionPasses

[makslevental]

There's a bug in the way Conversion.capi (and Transforms.capi) is generated; without the CIRCT prefix there's a collision with upstream's registerConversionPasses (and registerTransformsPasses) which causes an ODR violation if one actually tries to link in upstream's registerConversionPasses (e.g., by adding MLIRPythonExtension.RegisterEverything to CIRCTBindingsPythonCAPI).

Note, it's not clear to me whether the intent was to use CIRCT's transform C APIs or in fact upstream's transform C APIs so I added CIRCTCAPITransforms and added a corresponding call to mlirRegisterCIRCTTransformsPasses in CIRCTModule.cpp.

[mikeurbach]

Thanks for this, it's been a while since I thought about the generated C API stuff, but it makes sense we were generating conflicting things (probably copy pasted from upstream without thinking about it). I think we only intend to include CIRCT's conversion and transform libs in the Python module; the only thing we want from upstream is the "core" library:

circt/lib/Bindings/Python/CMakeLists.txt

Line 243 in cdd3814

MLIRPythonSources.Core

[mikeurbach]

I think we can remove the upstream mlirRegisterTransformsPasses, like mlirRegisterConversionPasses.

[makslevental]

Done

[makslevental]

Looks like that was load bearing for cse. I think you can just do registerCSE but I'm wondering if there are any other such pass uses and whether adding them one by one is worth it. Your call @mikeurbach .

[mikeurbach]

Ah, hmm, I guess we should just add back mlirRegisterTransformsPasses if it was load bearing, sorry for the churn. Now that I look again, we did explicitly link the upstream lib, presumably for this:

circt/lib/Bindings/Python/CMakeLists.txt

Line 54 in f17af93

MLIRCAPITransforms

So if I understand, this means we were actually registering the upstream transforms here previously, but not CIRCT's transforms? I guess no one needed to use one of CIRCT's transforms or else they'd have noticed this.

[makslevental]

That's not registration that's just linkage right - the mlirRegisterTransformsPasses does the actual registration (MLIRPythonSources.RegisterEverything is a different story because it does a lot of stuff on initialization). But yea prior to the rename and me adding CAPI/Transforms.h that call was definitely going to upstream's mlirRegisterTransformsPasses, not CIRCT's (even though they were named the same).

I guess no one needed to use one of CIRCT's transforms or else they'd have noticed this.

That's my read on it too. But there's no reason to preserve this flaw I think? I dunno do y'all have internal users that might be depending on mlirRegisterTransformsPasses being upstream's in a way that's not reflected in the tests here in OSS?

[mikeurbach]

Any use cases for running passes through the Python bindings should be tested in the OSS repo here, like the failure you encountered. So I think it was just the CSE pass we seemed to care about, and what's in this PR looks good to me.

[makslevental]

Cool then I'll narrow the registration calls (MLIRCAPITransforms is actually needed for mlirFrozenRewritePattern*; thanks @jpienaar!).