Page MenuHomePhabricator

Add RegionBranchOpInterface on affine.for op
ClosedPublic

Authored by bondhugula on Apr 11 2022, 9:20 PM.

Details

Summary

Add RegionBranchOpInterface on affine.for op so that transforms relying
on RegionBranchOpInterface can support affine.for. E.g.:
buffer-deallocation pass.

Diff Detail

Event Timeline

bondhugula created this revision.Apr 11 2022, 9:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2022, 9:20 PM
bondhugula requested review of this revision.Apr 11 2022, 9:20 PM

Base it on the right commit.

ftynse added inline comments.Apr 12 2022, 12:49 AM
mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
345–350

You can do DeclareOpInterfaceMethods<RegionBranchOpInterface, ["getSuccessorEntryOperands"]> in the declaration to request declarations to be emitted for additional interface methods.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp
1746–1748

I am a bit rusty on what the interface expects here, so just pointing out that the loop may have zero iterations and branch into the body.

Thanks for adding this.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp
1746–1748

That is a great observation and this is wrong for the scf::ForOp, as well. In both cases (so independent of index value) the loop might branch into the body or return to the parent. We could be clever here and if the loop is known to have at least one trip, omit the immediate return to the body. IIRC there is some helper to reason about trip counts.

bondhugula marked 2 inline comments as done.

Thanks for the review, @ftynse, @herhut. Fixed and enhanced the check. Tested via SCCP as well.

@ftynse @herhut -- any other comments here? Everything's addressed.

herhut accepted this revision.Apr 19 2022, 4:25 AM

Thanks!

This revision is now accepted and ready to land.Apr 19 2022, 4:25 AM
This revision was landed with ongoing or failed builds.Apr 20 2022, 5:19 AM
This revision was automatically updated to reflect the committed changes.