This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Linalg] NFC - Factor out Linalg functionality for shape and loop bounds computation
ClosedPublic

Authored by nicolasvasilache on Nov 20 2020, 4:51 AM.

Details

Summary

This revision refactors code used in various Linalg transformations and makes it a first class citizen to the LinalgStructureOpInterface. This is in preparation to allowing more advanced Linalg behavior but is otherwise NFC.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Nov 20 2020, 4:51 AM

Simplify hooks.

bondhugula requested changes to this revision.Nov 21 2020, 8:20 PM
bondhugula added a subscriber: bondhugula.
bondhugula added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOpsInterface.td
776

Typo

798

Rephrase "used to the".

874

It's not really "operand dimensions" but "operand dimension sizes".

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
13–16

Includes not in sorted order. You also don't need many of the includes below as a result.

18–19

You don't need this either.

18–19

You don't need this include.

18–19

Not needed either.

21–22

Not needed.

21–22

Not needed. Prune these includes overall.

33

Doc comment missing.

65

Use i = 0, e = ... form to avoid repeated evaluation.

71

if (!attr) and early return. Will reduce indent / more readable for the entire block below.

128–129

Use .isa<AffineConstantExpr>() / isa<AffineSymbolExpr>() instead of getKind() comparison.

mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp
335–337

Avoid auto here.

This revision now requires changes to proceed.Nov 21 2020, 8:20 PM
nicolasvasilache marked 14 inline comments as done.

Address

mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
13–16

Seems commonplace to isolate the matching include when such a unique include exists.
See e.g. PDL.cpp

128–129

This is moved code that will be deleted in the next revision.
But sure, thanks.

ftynse accepted this revision.Nov 23 2020, 1:17 AM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 23 2020, 2:21 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.