This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Move the Builtin FuncOp to the Func dialect
ClosedPublic

Authored by rriddle on Mar 8 2022, 5:22 PM.

Details

Summary

This commit moves FuncOp out of the builtin dialect, and into the Func
dialect. This move has been planned in some capacity from the moment
we made FuncOp an operation (years ago). This commit handles the
functional aspects of the move, but various aspects are left untouched
to ease migration: func::FuncOp is re-exported into mlir to reduce
the actual API churn, the assembly format still accepts the unqualified
func. These temporary measures will remain for a little while to
simplify migration before being removed.

Diff Detail

Event Timeline

rriddle created this revision.Mar 8 2022, 5:22 PM
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a reviewer: jpienaar. · View Herald Transcript
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
rriddle requested review of this revision.Mar 8 2022, 5:22 PM

Can you rebase this patch?

mlir/test/lib/Dialect/Affine/TestAffineDataCopy.cpp
26

Oh that was just unused? This isn't related to this patch though right?

Can you rebase this patch?

Maybe the whole chain of patches, I can't arc patch it right now unfortunately.

rriddle updated this revision to Diff 415330.Mar 15 2022, 12:06 AM
rriddle edited the summary of this revision. (Show Details)
rriddle updated this revision to Diff 415649.Mar 15 2022, 5:41 PM
mehdi_amini accepted this revision.Mar 16 2022, 12:57 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 16 2022, 5:15 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.
thopre added a subscriber: thopre.Apr 1 2022, 1:29 PM

Note: there is still a few reference to mlir::FuncOp in the tree: most notably in the toy example:

mlir/examples/toy/Ch5/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch5/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/examples/toy/Ch6/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch6/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/examples/toy/Ch7/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch7/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp:  AsmState state(padOp->getParentOfType<mlir::FuncOp>());
mlir/test/lib/Dialect/Test/TestOps.td:  let builders = [OpBuilder<(ins "::mlir::FuncOp":$function)>];

Note: there is still a few reference to mlir::FuncOp in the tree: most notably in the toy example:

mlir/examples/toy/Ch5/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch5/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/examples/toy/Ch6/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch6/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/examples/toy/Ch7/mlir/LowerToAffineLoops.cpp:    auto func = rewriter.create<mlir::FuncOp>(op.getLoc(), op.getName(),
mlir/examples/toy/Ch7/toyc.cpp:    mlir::OpPassManager &optPM = pm.nest<mlir::FuncOp>();
mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp:  AsmState state(padOp->getParentOfType<mlir::FuncOp>());
mlir/test/lib/Dialect/Test/TestOps.td:  let builders = [OpBuilder<(ins "::mlir::FuncOp":$function)>];

Yeah, thanks. I haven't gotten around to actually updating the in-tree references, this commit was just the minimal set to get
everything to build after the move.

mlir/test/lib/Dialect/Affine/TestAffineDataCopy.cpp
26

Yep, also good point (will remove in an NFC patch before landing this).