This is an archive of the discontinued LLVM Phabricator instance.

[mlir][transform] Fix apply WithPDLPatternsOp with non-pattern op
ClosedPublic

Authored by sotoshigoto on Feb 7 2023, 12:43 AM.

Details

Summary

Fix https://github.com/llvm/llvm-project/issues/60209

Fix crash with segmentation fault when transform::WithPDLPatternsOp is
applied with non-pattern op. Added check for existing transform ops with
pattern op.

Diff Detail

Event Timeline

sotoshigoto created this revision.Feb 7 2023, 12:43 AM
sotoshigoto requested review of this revision.Feb 7 2023, 12:43 AM
ftynse requested changes to this revision.Feb 7 2023, 12:50 AM

Thanks!

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

Nit: expand auto.

1014–1015

Why not reporting the error when there is a parent that is possibly a top-level op? WithPDLPatterns is always expected to have one non-pattern child.

This revision now requires changes to proceed.Feb 7 2023, 12:50 AM
sotoshigoto added inline comments.Feb 7 2023, 1:27 AM
mlir/lib/Dialect/Transform/IR/TransformOps.cpp
1014–1015

I think it can pass when there is a parent that may be a top-level op from the below test case.
https://github.com/llvm/llvm-project/blob/main/mlir/test/Dialect/Transform/ops.mlir#L26

But I will fix it.

ftynse added a comment.Feb 7 2023, 1:30 AM

That would trigger the exact same error as this patch is attempting to fix. Until this patch, we would allow "empty" WithPDLPatterns at the verifier level, but it could lead to crashes when applying the transformation. We can either disallow them, or fix the application so it does nothing.

ftynse accepted this revision.Feb 7 2023, 1:32 AM
This revision is now accepted and ready to land.Feb 7 2023, 1:32 AM
sotoshigoto marked 2 inline comments as done.Feb 8 2023, 5:11 PM

@ftynse I don't have commit access. Could you please commit to this patch?