This is an archive of the discontinued LLVM Phabricator instance.

[mlir][SCF] Add utility to outline the then and else branches of an scf.IfOp
ClosedPublic

Authored by nicolasvasilache on Aug 6 2020, 10:34 AM.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2020, 10:34 AM
nicolasvasilache requested review of this revision.Aug 6 2020, 10:34 AM
jurahul added inline comments.Aug 6 2020, 10:48 AM
mlir/lib/Dialect/SCF/Transforms/Utils.cpp
84

A utility to outline just a single Region will be useful by itself in other contexts. Can this be hoisted out to a common place (outside SCF dialect)?

mlir/lib/Dialect/SCF/Transforms/Utils.cpp
84

Yes, but this will require handling multiple blocks which I will not have cycles to generalize before going on vacation.
At this point you may be better off refactoring / moving some code around once this lands if you need it?

rriddle added inline comments.Aug 6 2020, 11:33 AM
mlir/lib/Dialect/SCF/Transforms/Utils.cpp
109

This looks weird, can you just remap after construction or something? Cloning shouldn't be something considered this cheap.

nicolasvasilache marked 2 inline comments as done.Aug 7 2020, 3:08 AM

Drop extra clone.

ftynse accepted this revision.Aug 7 2020, 4:41 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/SCF/Utils.h
57

Typo: "thepointer"

mlir/lib/Dialect/SCF/Transforms/Utils.cpp
90

Should the function rather take an OpBuilder as argument? This will make it compatible with the pattern rewriting infra.

96

What's the problem specifically? values.getTypes() should give you a TypeRange, and FunctionType::get accepts it.

106

Could this be simplified to for (Operation &op : ifOrElseRegion.front().without_terminator()) and a separate handling of the terminator?

111

If you lookup operands and put them in a vector, which is then passed into b.create, this will become compatible with pattern rewriting.

This revision is now accepted and ready to land.Aug 7 2020, 4:41 AM
nicolasvasilache marked 3 inline comments as done.

Address review.

nicolasvasilache marked 2 inline comments as done.

DCE.

mlir/lib/Dialect/SCF/Transforms/Utils.cpp
96

Wouldn't convert automatically when this was originally written, works with rebase.

This revision was landed with ongoing or failed builds.Aug 7 2020, 11:50 AM
This revision was automatically updated to reflect the committed changes.