This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Linalg][Vector] Add forwarding patterns between linalg.copy and vector.transfer
ClosedPublic

Authored by nicolasvasilache on May 28 2020, 8:35 AM.

Details

Summary

This revision adds custom rewrites for patterns that arise during linalg structured
ops vectorization. These patterns allow the composition of linalg promotion,
vectorization and removal of redundant copies.

The patterns are voluntarily limited and restrictive atm.
More robust behavior will be implemented once more powerful side effect modeling and
analyses are available on view/subview.

On the transfer_read side, the following pattern is rewritten:

%alloc = ...
[optional] %view = std.view %alloc ...
%subView = subview %allocOrView ...
[optional] linalg.fill(%allocOrView, %cst) ...
...
linalg.copy(%in, %subView) ...
vector.transfer_read %allocOrView[...], %cst ...

into

[unchanged] %alloc = ...
[unchanged] [optional] %view = std.view %alloc ...
[unchanged] [unchanged] %subView = subview %allocOrView ...
...
vector.transfer_read %in[...], %cst ...

On the transfer_write side, the following pattern is rewriten:

%alloc = ...
[optional] %view = std.view %alloc ...
%subView = subview %allocOrView...
...
vector.transfer_write %..., %allocOrView[...]
linalg.copy(%subView, %out)

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2020, 8:35 AM
rriddle added inline comments.May 28 2020, 11:22 AM
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
162

I don't think this namespace is necessary.

184

nit: Drop else after return.

aartbik added inline comments.May 28 2020, 9:29 PM
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
459

and missing

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
133

perhaps give it a comment:

Test for interleaved uses, conservatively erring on their existence

or something to show the conservative assumption (the may part)

193

without interleaved .... ?

229

comment reads a bit awkward...

261

as requested above by riddle

ftynse accepted this revision.May 29 2020, 4:11 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h
454

Nit: %A is not defined here, did you mean {%alloc or %view}. Same in the commit description.

458

Could you also mention what is this pattern rewritten _to_ ?

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
106

I wonder if we want to write some macro wrapper along the lines of

#ifdef DEBUG_TYPE
#define DBGS() \
  (dbgs() << '[' << DEBUG_TYPE << "] ")
#else
#define DBGS() \
  (dbgs())
#endif

and propose it for LLVM.

140

Please document the precondition somwhere

168

I don't understand this comment

261

Maybe this can be factored out into a function?

This revision is now accepted and ready to land.May 29 2020, 4:11 AM
nicolasvasilache marked 7 inline comments as done.

Address review + cleanups.

nicolasvasilache marked 7 inline comments as done.May 29 2020, 4:57 AM
nicolasvasilache added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
106

It hasn't bugged me enough to go and do it but it would make sense.

nicolasvasilache marked an inline comment as done.

Address review.

nicolasvasilache edited the summary of this revision. (Show Details)May 29 2020, 5:09 AM
This revision was automatically updated to reflect the committed changes.

I believe this change broke the windows buildbot as well, just as an additional data point. http://lab.llvm.org:8011/builders/mlir-windows/builds/2485

1025.187 [461/64/2346] Building CXX object tools\mlir\lib\Dialect\Linalg\Transforms\CMakeFiles\obj.MLIRLinalgTransforms.dir\Loops.cpp.obj
FAILED: tools/mlir/lib/Dialect/Linalg/Transforms/CMakeFiles/obj.MLIRLinalgTransforms.dir/Loops.cpp.obj 
C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -DMLIR_ROCM_CONVERSIONS_ENABLED=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\mlir\lib\Dialect\Linalg\Transforms -IE:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\lib\Dialect\Linalg\Transforms -Iinclude -IE:\build_slave\mlir-x64-windows-ninja\llvm-project\llvm\include -IE:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\include -Itools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2    /EHs-c- /GR- -UNDEBUG -std:c++14 /showIncludes /Fotools\mlir\lib\Dialect\Linalg\Transforms\CMakeFiles\obj.MLIRLinalgTransforms.dir\Loops.cpp.obj /Fdtools\mlir\lib\Dialect\Linalg\Transforms\CMakeFiles\obj.MLIRLinalgTransforms.dir\ /FS -c E:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\lib\Dialect\Linalg\Transforms\Loops.cpp
E:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/PatternMatch.h(192): error C2664: 'mlir::LogicalResult mlir::OpRewritePattern<mlir::vector::TransferWriteOp>::match(SourceOp) const': cannot convert argument 1 from 'SourceOp' to 'mlir::Operation *'
        with
        [
            SourceOp=mlir::vector::TransferWriteOp
        ]
E:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/PatternMatch.h(192): note: use of undefined type 'mlir::vector::TransferWriteOp'
E:\build_slave\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/Dialect/Linalg/Transforms/Transforms.h(20): note: see declaration of 'mlir::vector::TransferWriteOp'