This is an archive of the discontinued LLVM Phabricator instance.

[mlir] adapt TransformEachOpTrait to parameter values
ClosedPublic

Authored by ftynse on Jan 4 2023, 6:13 AM.

Details

Summary

Adapt the implementation of TransformEachOpTrait to the existence of
parameter values recently introduced into the transform dialect. In
particular, allow applyToOne hooks to return a list containing a mix
of Operation * that will be associated with handles and Attribute
that will be associated with parameter values by the trait
implementation of the transform interface's apply method.

Disentangle the "transposition" of the list of per-payload op partial
results to decrease its overall complexity and detemplatize the code
that doesn't really need templates. This removes the poorly documented
special handling for single-result ops with TransformEachOpTrait that
could have assigned null pointer values to handles.

Diff Detail

Event Timeline

ftynse created this revision.Jan 4 2023, 6:13 AM
ftynse requested review of this revision.Jan 4 2023, 6:13 AM
Herald added a project: Restricted Project. · View Herald Transcript
springerm accepted this revision.Jan 5 2023, 2:53 AM
springerm added inline comments.
mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
544–545

can have arbitrary number of input params?

545–546

operation handles and params?

738

How does this work? Attribute is not a pointer.

769

Why is this useful? Also, is this adding an nullptr operation* or a null attribute?

793

Does this class own the attributes or just pointers to them? Could there be an object lifetime issue?

798

When is null useful as a result?

835

new ops and params.

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

get asserts. Do you want dyn_cast here? Alternative: params.push_back(...get<Attribute>())

493

get asserts. Do you want dyn_cast here? Alternative: payloads.push_back(...get<Operation *>())

This revision is now accepted and ready to land.Jan 5 2023, 2:53 AM
ftynse updated this revision to Diff 486592.Jan 5 2023, 8:38 AM
ftynse marked 9 inline comments as done.

Address review.

mlir/include/mlir/Dialect/Transform/IR/TransformInterfaces.h
738
769

Good point, it looks useless now that I finished the refactoring.

793

Attributes are owned by the MLIRContext, as usual. We usually consider them immortal.

798

I don't think they are, but this was an allowed behavior before so I prefer to change it separately.

This revision was automatically updated to reflect the committed changes.