Page MenuHomePhabricator

[MLIR][Linalg] Hoist padding across multiple levels of tiling
ClosedPublic

Authored by nicolasvasilache on Mar 19 2021, 10:28 AM.

Details

Summary

This revision introduces proper backward slice computation during the hoisting of
PadTensorOp. This allows hoisting padding even across multiple levels of tiling.
Such hoisting requires the proper handling of loop bounds that may depend on enclosing
loop variables.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Mar 19 2021, 10:28 AM
mehdi_amini added inline comments.Mar 19 2021, 9:59 PM
mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
518

debugging leftover

519

Is this something that can be tripped with valid input IR? It isn't clear to me how this invariant is guaranteed by the structure of the callers right now?

571

(ditto)

Address review.

Address review.

nicolasvasilache marked 3 inline comments as done.Mar 22 2021, 7:41 AM
ftynse added inline comments.Mar 22 2021, 8:51 AM
mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
512

s/upper/lower

549–550

This feels like it can lead to invalid IR being produced. Just skipping over an op with regions that produces value will fail to create new values for these ops, and the old values will be used before being defined. I suppose we want to bail out completely. In this case, we also don't want to create ops spuriously, so I would suggest factoring out the validity check (no regions + no side effects) into a separate block/function that runs before we start creating new ops.

552–554

I suppose we shouldn't blindly clone side-effecting ops.

mlir/test/Dialect/Linalg/hoist-padding.mlir
3

Nit: passing -mlir-print-local-scope will print this inline and make tests more self-contained

nicolasvasilache marked 3 inline comments as done.Mar 23 2021, 10:33 AM
ftynse accepted this revision.Mar 23 2021, 10:36 AM
This revision is now accepted and ready to land.Mar 23 2021, 10:36 AM
This revision was landed with ongoing or failed builds.Mar 23 2021, 10:49 AM
This revision was automatically updated to reflect the committed changes.