Details

Reviewers

ftynse
mehdi_amini

Commits

rG3a2ff982d78f: Support post-processing Ops in unrolled loop iterations

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

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	310 ms	x64 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 ms	x64 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

Herald added subscribers: wrengr, Chia-hungDuan, dcaballe and 18 others. · View Herald TranscriptAug 9 2021, 3:42 PM

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

Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2021, 3:42 PM

Herald added subscribers: stephenneuendorffer, nicolasvasilache. · View Herald Transcript

Harbormaster completed remote builds in B118765: Diff 365305.Aug 9 2021, 4:18 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?

Addressed comments.

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

Thank you for the review! Can this be committed?

Harbormaster completed remote builds in B118919: Diff 365518.Aug 10 2021, 10:39 AM

In D107789#2937367, @nimiwio wrote:

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.

Minified test

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

In D107789#2938910, @mehdi_amini wrote:

In D107789#2937367, @nimiwio wrote:

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.

Harbormaster completed remote builds in B119096: Diff 365784.Aug 11 2021, 10:19 AM

Fix test

This revision was landed with ongoing or failed builds.Aug 11 2021, 4:11 PM

Closed by commit rG3a2ff982d78f: Support post-processing Ops in unrolled loop iterations (authored by nimiwio, committed by mehdi_amini). · Explain Why

This revision was automatically updated to reflect the committed changes.

mehdi_amini added a commit: rG3a2ff982d78f: Support post-processing Ops in unrolled loop iterations.

Harbormaster completed remote builds in B119156: Diff 365865.Aug 11 2021, 4:11 PM

ThomasRaoux mentioned this in D119866: [mlir][scf] Add callback to annotate ops during pipelining.Feb 15 2022, 11:29 AM