Page MenuHomePhabricator

Support post-processing Ops in unrolled loop iterations
ClosedPublic

Authored by nimiwio on Aug 9 2021, 3:42 PM.

Details

Summary

This can be useful when one needs to know which unrolled iteration an Op belongs to, for example, conveying noalias information among memory-affecting ops in parallel-access loops.

Diff Detail

Unit TestsFailed

TimeTest
310 msx64 debian > MLIR.Dialect/SCF::loop-unroll.mlir
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/mlir-opt /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Dialect/SCF/loop-unroll.mlir -test-loop-unrolling='unroll-factor=2' | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/mlir/test/Dialect/SCF/loop-unroll.mlir --check-prefix UNROLL-BY-2
290 msx64 windows > MLIR.Dialect/SCF::loop-unroll.mlir
Script: -- : 'RUN: at line 1'; c:\ws\w8\llvm-project\premerge-checks\build\bin\mlir-opt.exe C:\ws\w8\llvm-project\premerge-checks\mlir\test\Dialect\SCF\loop-unroll.mlir -test-loop-unrolling='unroll-factor=2' | c:\ws\w8\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w8\llvm-project\premerge-checks\mlir\test\Dialect\SCF\loop-unroll.mlir --check-prefix UNROLL-BY-2

Event Timeline

nimiwio created this revision.Aug 9 2021, 3:42 PM
nimiwio requested review of this revision.Aug 9 2021, 3:42 PM

LGTM overall

mlir/include/mlir/Transforms/LoopUtils.h
44

Not sure about this TODO, can you just fill the doc here?

mlir/lib/Transforms/Utils/LoopUtils.cpp
1088

Nit: remove spurious braces

mlir/test/lib/Transforms/TestLoopUnrolling.cpp
50

I'm not sure I get this TODO?

nimiwio updated this revision to Diff 365518.Aug 10 2021, 9:42 AM
nimiwio marked 2 inline comments as done.

Addressed comments.

mlir/test/lib/Transforms/TestLoopUnrolling.cpp
50

Removed.

Thank you for the review! Can this be committed?

mehdi_amini accepted this revision.Aug 11 2021, 1:08 AM

Thank you for the review! Can this be committed?

To clarify: are you asking me to commit for you (don't have commit access) or do you ask confirmation that you can land it now?

mlir/test/Dialect/SCF/loop-unroll.mlir
198

One nit: in general I prefer targeted testing.
Meaning we could write minimally:

// UNROLL-BY-2-ANNOTATE: memref.store {{.*}} unrolled_iteration = 0
// UNROLL-BY-2-ANNOTATE: memref.store {{.*}} unrolled_iteration = 1

The rest is already tested in other tests.

This revision is now accepted and ready to land.Aug 11 2021, 1:08 AM
nimiwio updated this revision to Diff 365783.Aug 11 2021, 9:52 AM
This comment was removed by nimiwio.
nimiwio updated this revision to Diff 365784.Aug 11 2021, 9:55 AM

Minified test

nimiwio marked 2 inline comments as done.Aug 11 2021, 9:57 AM

Thank you for the review! Can this be committed?

To clarify: are you asking me to commit for you (don't have commit access) or do you ask confirmation that you can land it now?

I guess both.. This is my first commit (so I definitely don't have commit access) and I'm also unsure of what amount of review/consensus sufficient to land a change. If it's okay to commit now, that would be good with me.

This revision was landed with ongoing or failed builds.Aug 11 2021, 4:11 PM
This revision was automatically updated to reflect the committed changes.