This is an archive of the discontinued LLVM Phabricator instance.

[mlir][transform] Add ForeachOp to transform dialect
ClosedPublic

Authored by springerm on Jul 21 2022, 5:27 AM.

Details

Summary

This op "unbatches" an op handle and executes the loop body for each payload op.

Depends On D130244

Diff Detail

Event Timeline

springerm created this revision.Jul 21 2022, 5:27 AM
springerm requested review of this revision.Jul 21 2022, 5:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2022, 5:27 AM
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
290

Seems dangerous to potentially fail silently if the IR may be left in an invalid state

mlir/test/Dialect/Transform/ops.mlir
61

nit: nl

springerm added inline comments.Jul 22 2022, 2:15 AM
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
290

The transform may fail silently but it still fails. In that case, the entire ForeachOp will fail (silently). It is my understanding that this will "silent failure" will continue to bubble up until it is caught by an op that handles the failure (e.g., the alternatives op, by restoring the pre-transform IR). If no op handles the failure, the entire transform will fail.

So I think it is OK to fail silently here and we do not have to worry about invalid IR.

nicolasvasilache accepted this revision.Jul 22 2022, 2:17 AM
This revision is now accepted and ready to land.Jul 22 2022, 2:17 AM
springerm updated this revision to Diff 446755.Jul 22 2022, 2:34 AM
springerm marked 2 inline comments as done.

address comments

I would consider describing the default "batched" mode in https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Transform/IR/TransformDialect.td and mentioning this operation as a way of obtaining "unbatched" mode.

mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
124

I would consider returning as many handles as transform.yield has operands, with each handle corresponding to a list of individual payload operations yielded by each iteration. This would make this op composable with functional-style composition of transformations. Not necessary for this commit.

mlir/lib/Dialect/Transform/IR/TransformOps.cpp
290

Indeed. One of the big points of DiagnosedSilenceableFailure is to report errors in the place that has the most information and just bubble up the error. Therefore, container ops such as this one are not supposed to add extra error messages, leaf ops are, or even better the actual transformation code.

mlir/test/Dialect/Transform/test-interpreter.mlir
604–608

We need a mechanism to check that transformations are indeed applied to one op at a time. Here, the output of the "batched" mode will be indistinguishable. We have TestPrintNumberOfAssociatedPayloadIROps that could help here.

ftynse accepted this revision.Jul 26 2022, 6:52 AM
springerm updated this revision to Diff 447735.Jul 26 2022, 9:07 AM
springerm marked an inline comment as done.

address comments

This revision was landed with ongoing or failed builds.Jul 26 2022, 9:11 AM
This revision was automatically updated to reflect the committed changes.