This is an archive of the discontinued LLVM Phabricator instance.

Fix conversion of loops to GPU with no block/thread dimensions.
ClosedPublic

Authored by herhut on Jan 30 2020, 2:38 AM.

Details

Summary

The current code assumes that one always maps at least one loop to block
dimensions and at least one loop to thread dimensions. If either is not
the case, a loop would get mapped twice.

Diff Detail

Event Timeline

herhut created this revision.Jan 30 2020, 2:38 AM
herhut edited reviewers, added: ftynse; removed: nicolasvasilache.Jan 30 2020, 2:38 AM
ftynse accepted this revision.Jan 30 2020, 2:44 AM

Good catch, thanks!

mlir/test/Conversion/LoopsToGPU/no_blocks_no_threads.mlir
12

Nit: do we really need to know there are two "constant 1" emitted?
More nit: does the absence of ops in between the given ops (-NEXT) matter?

This revision is now accepted and ready to land.Jan 30 2020, 2:44 AM

Unit tests: pass. 62328 tests passed, 0 failed and 838 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

herhut marked an inline comment as done.Jan 30 2020, 3:38 AM
herhut added inline comments.
mlir/test/Conversion/LoopsToGPU/no_blocks_no_threads.mlir
12

I took the NEXT from the other loop lowering test. The check for two constant 1 is there as we have two constants and I need to match the second. I want to make sure that unmapped grids/blocks actually are constant one. Is there a better way?

ftynse added inline comments.Jan 30 2020, 3:41 AM
mlir/test/Conversion/LoopsToGPU/no_blocks_no_threads.mlir
12

I see. It's fine this way.

This revision was automatically updated to reflect the committed changes.