This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Enable fusion on conv op if it is a consumer.
AbandonedPublic

Authored by hanchung on Apr 1 2020, 9:24 PM.

Details

Summary

The fuse method will tile the producer based on the subview of the operand in
the consumer. Thus, we don't really care if the consumer has padding or not. It
only breaks if the conv op is a producer (and has padding).

Add a comment for fuse method as well.

Diff Detail

Event Timeline

hanchung created this revision.Apr 1 2020, 9:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2020, 9:24 PM
bondhugula requested changes to this revision.Apr 1 2020, 9:53 PM
bondhugula added a subscriber: bondhugula.

Could you add a test case?

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
140

Doc comment ///

154

You've also removed this TODO comment - check if it's still applicable.

This revision now requires changes to proceed.Apr 1 2020, 9:53 PM
bondhugula resigned from this revision.Apr 1 2020, 9:54 PM
mravishankar requested changes to this revision.Apr 1 2020, 10:55 PM

I am not convinced that this is right. The issue with the padding is that you cannot have a subview with padding which can correctly represent the padding in the interior as well as the boundaries. Without going into details, the conv op on subviews is the padded version along the edges, and is the unpadded version in the interior.
The best you can do here is that you can check that the subview is not a result of tiling along the "x" loops of the output, i.e. the size of the subview matches the size of the tensor. You can still tile and fuse along the batch dimension.

nicolasvasilache requested changes to this revision.Apr 3 2020, 8:01 AM

As Mahesh says, this has incorrect assumptions.
Even just tiling of a conv. op along padded dimensions is not a closed operation and cannot be represented with a single op + attribute.

hanchung abandoned this revision.Jul 13 2020, 9:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2020, 9:37 AM