Page MenuHomePhabricator

[MLIR] Fix permuteLoops utility
ClosedPublic

Authored by bondhugula on Mar 28 2020, 7:06 PM.

Details

Summary

Rewrite mlir::permuteLoops for affine loop permutation to fix incorrect
approach. Avoiding using sinkLoops entirely - use single move approach.
Add test pass.

This fixes https://bugs.llvm.org/show_bug.cgi?id=45328

Depends on D77003.

Signed-off-by: Uday Bondhugula <uday@polymagelabs.com>

Diff Detail

Unit TestsFailed

TimeTest
110 msClang.Driver::Unknown Unit Message ("")
Script: -- : 'RUN: at line 7'; printf '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Driver/cl-response-file.c\n' '/I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Driver\Inputs\cl-response-file\ /DFOO=2' > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/tools/clang/test/Driver/Output/cl-response-file.c.tmp.rsp
20 msLLVM.Linker::Unknown Unit Message ("")
Script: -- : 'RUN: at line 4'; echo "/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Outputy = type opaque @GV = external global /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Outputy*" | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/llvm-as > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output/2003-01-30-LinkerTypeRename.ll.tmp.1.bc
30 msLLVM.Linker::Unknown Unit Message ("")
Script: -- : 'RUN: at line 4'; echo "/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output = type i32" | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/llvm-as > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output/2003-04-26-NullPtrLinkProblem.ll.tmp.2.bc
20 msLLVM.Linker::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; echo "/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output = type opaque" | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/llvm-as > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output/2003-06-02-TypeResolveProblem.ll.tmp.2.bc
30 msLLVM.Linker::Unknown Unit Message ("")
Script: -- : 'RUN: at line 1'; echo "/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output = type i32" | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/bin/llvm-as > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/test/Linker/Output/2003-06-02-TypeResolveProblem2.ll.tmp.1.bc
View Full Test Results (8 Failed)

Event Timeline

bondhugula created this revision.Mar 28 2020, 7:06 PM

Update child revision

andydavis1 accepted this revision.Mar 30 2020, 7:54 AM

Thanks for fixing this bug Uday!

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

Can you add some comments describing what is going on here? having 'input' indexed at a hard coded value of 0 needs explanation. Are you splicing ops from one loop before into the first loops parent block?

This revision is now accepted and ready to land.Mar 30 2020, 7:54 AM
bondhugula marked an inline comment as done.

Address review comment - add comment.

This revision was automatically updated to reflect the committed changes.

Thanks for addressing this, Uday!

mlir/test/Dialect/Affine/loop-permute.mlir
36

Nice! That's an interesting way to check that the loop order is the expected :)

mlir/test/Dialect/Affine/loop-permute.mlir
27

plz use captures instead of SSA names.

bondhugula marked 3 inline comments as done.Mar 30 2020, 3:12 PM
bondhugula added inline comments.
mlir/test/Dialect/Affine/loop-permute.mlir
27

This is a different/special scenario - see below the 3! permutations. We don't need to capture the loop IVs anywhere.