This is an archive of the discontinued LLVM Phabricator instance.

[mlir][PassIncGen] Refactor how pass registration is generated
ClosedPublic

Authored by rriddle on Jul 30 2020, 1:56 PM.

Details

Summary

The current output is a bit clunky and requires including files+macros everywhere, or manually wrapping the file inclusion in a registration function. This revision refactors the pass backend to automatically generate registerFooPass/registerFooPasses functions that wrap the pass registration. gen-pass-decls now takes a -name input that specifies a tag name for the group of passes that are being generated. For each pass, the generator now produces a registerFooPass where Foo is the name of the definition specified in tablegen. It also generates a registerGroupPasses, where Group is the tag provided via the -name input parameter, that registers all of the passes present.

Diff Detail

Event Timeline

rriddle created this revision.Jul 30 2020, 1:56 PM
rriddle requested review of this revision.Jul 30 2020, 1:56 PM

Thanks River!

So I just want to confirm that as a user if I call, say registerGpuKernelOutliningPass() I just include GPU/Passes.h and depend on MLIRGPU and don't end up bringing, say cuda, into my build. Right?

mlir/include/mlir/InitAllPasses.h
31–57

Wooooo! This is so much prettier. Thank you River :-)

mehdi_amini accepted this revision.Jul 30 2020, 4:03 PM
mehdi_amini added inline comments.
mlir/tools/mlir-tblgen/PassGen.cpp
161

I like the group name, something that we could preserve cheaply though is the ability to register individual passes: we just need to emit:

inline registerXYZPass() {
   ... 
}

And then the registration for the group is just a sequence of calls to these individual registration functions.

mehdi_amini added inline comments.Jul 30 2020, 4:04 PM
mlir/include/mlir/Dialect/Shape/Transforms/Passes.h
45

Something else for a future change would be to generate two files: one for the public API and the other for the pass declaration.
That would avoid to have to use this macro trick and make the generated files more self-contained.

GMNGeoffrey added inline comments.Jul 30 2020, 4:15 PM
mlir/tools/mlir-tblgen/PassGen.cpp
161

Maybe I missed something, but I thought that's exactly what this does. We get one function for each pass and then the registerGroupPass just calls them all

jpienaar accepted this revision.Jul 30 2020, 4:20 PM
jpienaar added inline comments.
mlir/docs/PassManagement.md
625–634

s/the much/much/

626

boilerplate

650

boilerplate

651

TableGen (or ODS)

652

Not sure what update refers to

671

s/of real MLIR passes//

mlir/include/mlir/InitAllPasses.h
44–45

These now doesn't seem to add much to me (I feel the function name is descriptive enough and readable) that I'd probably consider removing the comments and empty lines here.

mlir/tools/mlir-tblgen/PassGen.cpp
161

+1

This revision is now accepted and ready to land.Jul 30 2020, 4:20 PM

So I just want to confirm that as a user if I call, say registerGpuKernelOutliningPass() I just include GPU/Passes.h and depend on MLIRGPU and don't end up bringing, say cuda, into my build. Right?

If you register a pass that itself depends on cuda, you'll get some link error at some point (or if cuda is well configured in the build system, it'll pull in the binary transitively).

mlir/tools/mlir-tblgen/PassGen.cpp
161

Oh I missed the first loop, so indeed that's it :)

Thanks River!

So I just want to confirm that as a user if I call, say registerGpuKernelOutliningPass() I just include GPU/Passes.h and depend on MLIRGPU and don't end up bringing, say cuda, into my build. Right?

Yes, that is the intention at least.

rriddle marked 6 inline comments as done.Jul 31 2020, 12:28 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/Shape/Transforms/Passes.h
45

Yeah, I'd really love it if I could just generate multiple files from tablegen.

mlir/tools/mlir-tblgen/PassGen.cpp
161

Yep, the group one just contains calls to each of the individual ones.

rriddle updated this revision to Diff 282302.Jul 31 2020, 1:18 PM
rriddle marked 2 inline comments as done.

Fixup flang

This revision was landed with ongoing or failed builds.Jul 31 2020, 1:26 PM
This revision was automatically updated to reflect the committed changes.