This is an archive of the discontinued LLVM Phabricator instance.

[mlir][transform] Introduce transform.sequence op
ClosedPublic

Authored by ftynse on Apr 13 2022, 2:00 AM.

Details

Summary

Sequence is an important transform combination primitive that just indicates
transform ops being applied in a row. The simplest version requires fails
immediately if any transformation in the sequence fails. Introducing this
operation allows one to start placing transform IR within other IR.

Depends On D123135

Diff Detail

Event Timeline

ftynse created this revision.Apr 13 2022, 2:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 2:00 AM
ftynse requested review of this revision.Apr 13 2022, 2:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2022, 2:00 AM
ftynse updated this revision to Diff 422473.Apr 13 2022, 4:40 AM

More cmake.

Mogball added inline comments.Apr 13 2022, 10:06 PM
mlir/lib/Dialect/Transform/IR/TransformDialect.cpp
22

nit

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

editor struggles :(

mlir/test/Dialect/Transform/ops-invalid.mlir
53

nit

mlir/test/Dialect/Transform/ops.mlir
13

nit

ftynse updated this revision to Diff 422809.Apr 14 2022, 4:17 AM
ftynse marked 4 inline comments as done.

Address review.

ftynse updated this revision to Diff 422875.Apr 14 2022, 8:32 AM

Better documentation.

Mogball accepted this revision.Apr 18 2022, 6:14 PM
Mogball added inline comments.
mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
75

Why does this need to be wrapped in this def?

mlir/lib/Dialect/Transform/IR/TransformInterfaces.cpp
25

nit

This revision is now accepted and ready to land.Apr 18 2022, 6:14 PM
rriddle accepted this revision.Apr 18 2022, 6:23 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/Transform/IR/TransformDialect.td
161

Half written sentence?

mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
116–119

The general strategy is to const_cast and call the non-const, instead of duplicating the method.

128

Mappins?

129

Why not just friend TransformState?

166

Can you document this field? and the one below?

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

tranfsormation

mlir/lib/Dialect/Transform/IR/TransformOps.cpp
69
ftynse updated this revision to Diff 423619.Apr 19 2022, 7:37 AM
ftynse marked 8 inline comments as done.

Address review.

ftynse added inline comments.Apr 19 2022, 7:44 AM
mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
75

Because the definition of regionStack is also wrapped, and data members of a class are ABI-breaking (or at least quickly become such).

128

Muffins :)

129

Just a preference for tighter visibility control. I can change if you prefer otherwise.

opertaion -> operation in the summary.

ftynse edited the summary of this revision. (Show Details)Apr 19 2022, 12:39 PM
This revision was automatically updated to reflect the committed changes.