Page MenuHomePhabricator

[mlir][linalg] Retire Linalg ConvOp.
ClosedPublic

Authored by gysit on Oct 6 2021, 7:38 AM.

Details

Summary

The convolution op is one of the remaining hard coded Linalg operations that have no region attached. It got obsolete due to the OpDSL convolution operations. Removing it allows us to delete specialized code and tests that are not needed for the OpDSL counterparts that rely on the standard code paths.

Test needed due to specialized implementations are removed. Tiling and fusion tests are replaced by variants using linalg.conv_2d.

Diff Detail

Event Timeline

gysit created this revision.Oct 6 2021, 7:38 AM
gysit requested review of this revision.Oct 6 2021, 7:38 AM
nicolasvasilache accepted this revision.Oct 7 2021, 12:06 PM

Great! Thanks much for cleaning this up.

This revision is now accepted and ready to land.Oct 7 2021, 12:06 PM
gysit updated this revision to Diff 378105.Oct 7 2021, 11:38 PM

Rebase.

This revision was automatically updated to reflect the committed changes.
arnab-oss added subscribers: bondhugula, arnab-oss.EditedDec 19 2021, 11:14 PM

Initially Linalg::ConvOp had padding attribute, but the current set of convolution ops don't have them. Is there anyway I can define convolution with padding with the current Linalg Dialect?
CC - @bondhugula , @gysit , @nicolasvasilache

Initially Linalg::ConvOp had padding attribute, but the current set of convolution ops don't have them. Is there anyway I can define convolution with padding with the current Linalg Dialect?
CC - @bondhugula , @gysit , @nicolasvasilache

I think you cannot have a convolution with padding in one operation. However, you can combine a convolution op with a linalg.pad_tensor operation to emulate the old padding behavior.

Correct, padding is a separate first-class op now and there is no support for single-op conv+padding in linalg atm.
Once some of the ongoing refactorings are landed, it may make sense to introduce a frontend op that also allows padding as a convenience.