This is an archive of the discontinued LLVM Phabricator instance.

[mlir] [transform] Error for duplicated processor mapping
ClosedPublic

Authored by guraypp on Nov 15 2022, 6:52 AM.

Details

Summary

In a nested loop nest, it is not feasible to map different loops to the same processing unit; for an example, check the code below. This modification includes a check in this circumstance.

scf.foreach_thread (%i, %j) in (%c32, %c32) {...}
{ mapping = [#gpu.thread<x>, #gpu.thread<x>] }

Note: It also deletes a test because it is not possible to reproduce this error.

Depends on D138020

Diff Detail

Event Timeline

guraypp created this revision.Nov 15 2022, 6:52 AM
guraypp requested review of this revision.Nov 15 2022, 6:52 AM
guraypp updated this revision to Diff 475724.Nov 16 2022, 1:02 AM

rebase
add newline

ftynse added inline comments.Nov 16 2022, 12:44 PM
mlir/lib/Dialect/GPU/TransformOps/GPUTransformOps.cpp
46–48

The entire flow reads strange here. Assigning a captured variable inside a lambda inside the callback of any_of that kind of implies the function to be a non-side-effecting predicate, and then potentially overwriting it. Could we turn this into a normal loop with early returns instead?

48

Would llvm::is_contained work?

49–52

Nit: add braces around the conditional's body. We only omit braces when the statement is trivial, typically fits into one line.

guraypp updated this revision to Diff 476117.Nov 17 2022, 7:13 AM

address @ftynse comments

guraypp updated this revision to Diff 476121.Nov 17 2022, 7:16 AM

remove unnecessary include

ftynse accepted this revision.Nov 17 2022, 7:24 AM
ftynse added inline comments.
mlir/lib/Dialect/GPU/TransformOps/GPUTransformOps.cpp
39

Nit: ArrayRef is already an implicitly constant reference to an array, no need to pass it by const reference additionally.

This revision is now accepted and ready to land.Nov 17 2022, 7:24 AM
This revision was automatically updated to reflect the committed changes.