Page MenuHomePhabricator

Out-of-tree machine IR passes
Needs ReviewPublic

Authored by aguinet on Sep 5 2019, 11:40 PM.

Details

Summary

This patch add the possibilities to create out-of-tree MIR passes. This is based on how IR out-of-tree passes work.
If this is something that could be upstreamed, I could factorize the code between the two system, as they are very similar!

Diff Detail

Event Timeline

aguinet created this revision.Sep 5 2019, 11:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 5 2019, 11:40 PM

As a side remark, I have tested this with an out of tree MIR pass that dump MIR, and it works well. Should I add this test within the test suite? I didn't find such test for the current out-of-tree IR system (but chances are I missed it!).

Could you add some documentation, probably in docs/MIRLangRef.rst? Also you lack a test case, probably in ./CodeGen/MIR/Generic/

aguinet updated this revision to Diff 230379.Nov 20 2019, 11:25 PM

Thanks Serge for the comments !

I added examples, a test that test these examples, and a mention of them in the documentation (not sure it's in the right place though!).

aguinet updated this revision to Diff 230380.Nov 20 2019, 11:30 PM

Cleaning up some commented CMake code...

lkail added a subscriber: lkail.Nov 20 2019, 11:31 PM
lkail added inline comments.Nov 20 2019, 11:37 PM
examples/OutOfTreeMIR/ModulePass/ModulePass.cpp
2

Please add copyright information.

aguinet updated this revision to Diff 230383.Nov 21 2019, 12:14 AM

Copyright added on new cpp files.

serge-sans-paille added inline comments.
examples/CMakeLists.txt
10

Why don't you always build it?

examples/OutOfTreeMIR/FunctionPass/CMakeLists.txt
2

based on ../lib/Transforms/Hello/CMakeLists.txt, you could pass BUILDTREE_ONLY as an extra flag

lib/CodeGen/TargetPassConfig.cpp
229

This code is very similar to the legacy pass manager machinery, which is very different from the new PM one.

Adding the feature here totally makes sense to me, but I'm unsure if the MIR passes management is going to be reworked just like IR passes was, @chandlerc ?

aguinet marked an inline comment as done.Nov 25 2019, 5:23 AM
aguinet added inline comments.
examples/CMakeLists.txt
10

Because I test them as out-of-tree passes. My understanding is that, if LLVM isn't built with shared libraries, or with "LINK_LLVM_DYLIB", I won't be able to build a shared library for the passes that llc would load.

aguinet marked 2 inline comments as done.Nov 25 2019, 5:26 AM
aguinet added inline comments.
examples/OutOfTreeMIR/FunctionPass/CMakeLists.txt
2

Indeed. Will add it in the next patch!

lib/CodeGen/TargetPassConfig.cpp
229

That's a good question :)

aguinet updated this revision to Diff 231009.Nov 26 2019, 12:05 AM

Use BUILDTREE_ONLY, as suggested by @serge-sans-paille .

aguinet marked an inline comment as done.Nov 26 2019, 12:05 AM
aguinet marked an inline comment as not done.Dec 13 2019, 1:46 AM
aguinet added inline comments.
lib/CodeGen/TargetPassConfig.cpp
229

@chandlerc any insights of what will happen with the MIR passes management? Does this patch conflict with any current refactoring on the subject?

Thanks!