Page MenuHomePhabricator

[mlir][linalg] Fix vectorization of linalg depthwise conv for int types
AbandonedPublic

Authored by rsuderman on Nov 7 2022, 3:46 PM.

Details

Summary

Vectorization of Linalg's depthwise convolution only supports floating
point types. Previous version assumed floating point operations would
work. This version checks whether the computation is integer or floating
point and adjust the inner loop computation.

Diff Detail

Event Timeline

rsuderman created this revision.Nov 7 2022, 3:46 PM
Herald added a project: Restricted Project. · View Herald Transcript
rsuderman requested review of this revision.Nov 7 2022, 3:46 PM
rsuderman updated this revision to Diff 473816.Nov 7 2022, 3:47 PM

Reverted unnecessary change

hanchung accepted this revision.Nov 7 2022, 3:50 PM
hanchung added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
1774–1819

style nit: auto

This revision is now accepted and ready to land.Nov 7 2022, 3:50 PM
rsuderman updated this revision to Diff 473824.Nov 7 2022, 4:17 PM

Handled nit.

rsuderman marked an inline comment as done.Nov 7 2022, 4:18 PM
dcaballe accepted this revision.Nov 7 2022, 4:49 PM

LGTM! A couple of minor comments. Thanks!

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
1774
  • Add some doc?
  • Check that the dst type is actually wider?
rsuderman updated this revision to Diff 474098.Nov 8 2022, 3:01 PM

Fix for validating correct promot logic.

rsuderman updated this revision to Diff 474099.Nov 8 2022, 3:01 PM

git-clang-format

rsuderman marked an inline comment as done.Nov 8 2022, 3:02 PM
nicolasvasilache added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
1758

This is a problem: this is not a precondition and we have created I leaving IR in a potentially invalid state which will create all sorts of issues.

Please find a way to hoist that into a precondition and a guarantee that this will succeed on we start producing IR.

Separately:

if (!llvm::all_of(resVals, [](Value v){ return c}))
  return failure();
This revision is now accepted and ready to land.Nov 24 2022, 12:41 AM
nicolasvasilache requested changes to this revision.Nov 24 2022, 12:41 AM
This revision now requires changes to proceed.Nov 24 2022, 12:41 AM
rsuderman updated this revision to Diff 481734.Dec 9 2022, 1:02 PM

Reworked failure case.

rsuderman updated this revision to Diff 481735.EditedDec 9 2022, 1:04 PM

Removed failure case

rsuderman marked an inline comment as done.Dec 9 2022, 1:04 PM
rsuderman abandoned this revision.Dec 9 2022, 1:10 PM

Hey Rob, what happened with this diff?