This is an archive of the discontinued LLVM Phabricator instance.

Add lowering for loop.parallel to cfg.
ClosedPublic

Authored by herhut on Jan 24 2020, 6:20 AM.

Details

Summary

This also removes the explicit pattern for loop.terminator to ensure
that the terminator is only erased if the parent op is rewritten.

Reductions are not yet supported.

Diff Detail

Event Timeline

herhut created this revision.Jan 24 2020, 6:20 AM

Unit tests: fail. 62155 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

Looks generally okay to me. I don't see the point of cloning before erasing, but can live with it pending a clean-up.

mlir/lib/Conversion/LoopToStandard/ConvertLoopToStandard.cpp
275

Ultra-nit: drop trivial braces.

276

Have you considered moving the region instead of cloning (inlineRegionBefore) and then calling replaceAllUsesWith to update induction variables?

mlir/test/Conversion/convert-to-cfg.mlir
174

std looks like leftover from the temporary state when loops where in the standard dialect, let's drop it here (and later clean up the tests above)

herhut updated this revision to Diff 240555.Jan 27 2020, 6:27 AM
herhut marked 3 inline comments as done.

Review comments...

herhut added inline comments.Jan 27 2020, 6:32 AM
mlir/lib/Conversion/LoopToStandard/ConvertLoopToStandard.cpp
276

I did not want to rewrite the block arguments. In essence, I would need the equivalent of a signature conversion and that seemed involved. The issue here is that the block of the innermost loop only declares one induction variable.

mlir/test/Conversion/convert-to-cfg.mlir
174

I will clean it up and also move this test to LoopsToCfg as that test feels a bit misplaced.

ftynse accepted this revision.Jan 27 2020, 6:36 AM
This revision is now accepted and ready to land.Jan 27 2020, 6:36 AM

Unit tests: fail. 62155 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

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.

This revision was automatically updated to reflect the committed changes.