This is an archive of the discontinued LLVM Phabricator instance.

[mlir][tosa] Add broadcasting case for tosa.resize degenerate case
ClosedPublic

Authored by rsuderman on Oct 19 2022, 3:37 PM.

Details

Summary

When the resize is ?x1x1x?, the tosa.resize operation broadcasts the
input and (when quantized) applies a scaling factor. Updated the resize
operation to not use a tensor.extract operation, instead broadcasting
the only positional value as necessary.

Moved the tosa.resize tests to their own mlir test due to increased
complexity. Also corrected a bug where tosa.resize for bilinear-floating
point was not applying the correct scaling.

Diff Detail

Event Timeline

rsuderman created this revision.Oct 19 2022, 3:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2022, 3:37 PM
rsuderman requested review of this revision.Oct 19 2022, 3:37 PM
jpienaar added inline comments.Oct 20 2022, 11:36 AM
mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp
1325–1422

Why is this done during lowering to linalg vs a tosa->tosa pattern?

1345

No trailing punctuation, also feel free to omit tosa.resize here as the message would be reported along with the op.

1348

These should become enums eventually, perhaps add a TODO (or file github issue).

1351

Nit: camelCase naming convention here.

1371

Why are only these two checked for dynamic?

1612–1613

Perhaps change this to assert? (should be easy enough to read and give one extra layer of checking)

1712–1713

How did this work before without this ?

2326

degenerate

mlir/test/Conversion/TosaToLinalg/tosa-to-linalg-resize.mlir
381

CHECK-DAG for constants? (I know at least one spot where this'll get flagged :))

rsuderman marked 2 inline comments as done.

jpienaar@ comments

rsuderman marked 5 inline comments as done.

added assert

rsuderman marked 2 inline comments as done.Oct 20 2022, 12:17 PM
rsuderman added inline comments.
mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp
1325–1422

TOSA does not have a broadcast operations so there is no TOSA operation to lower to.

1371

We have already checked that the input width/height above are 1.

1712–1713

The replace was immediately after constructing the generic op. I moved it to one return at the end now.

jpienaar accepted this revision.Oct 20 2022, 12:56 PM

Thanks

mlir/test/Conversion/TosaToLinalg/tosa-to-linalg-resize.mlir
2

I don't see any diagnostics being verified.

71

(up to you) You could use the literal check here to avoid needing escaping

This revision is now accepted and ready to land.Oct 20 2022, 12:56 PM
rsuderman updated this revision to Diff 469335.Oct 20 2022, 1:27 PM

Fixed for use check{literal}

rsuderman updated this revision to Diff 469337.Oct 20 2022, 1:32 PM

remove diagnostic check

rsuderman marked 2 inline comments as done.Oct 20 2022, 2:15 PM