This is an archive of the discontinued LLVM Phabricator instance.

[mlir][MemRef][TransformOps] Fix error reporting for multibuffer
ClosedPublic

Authored by qcolombet on Feb 10 2023, 4:58 AM.

Details

Summary

Multibuffer will fail to apply on allocs that are used outside of loops.
This was properly caught in the current implementation but the way we report
it was broken.
Notes cannot be emitted on their own, they need to be attached to another
main diagnostic.

Long story short, change the severity of the report from Note to Remark.

Note: I'm making another patch to not even bother running the multibuffer transformation on such allocs

Diff Detail

Event Timeline

qcolombet created this revision.Feb 10 2023, 4:58 AM
qcolombet requested review of this revision.Feb 10 2023, 4:58 AM
qcolombet edited the summary of this revision. (Show Details)Feb 10 2023, 5:00 AM
ftynse added inline comments.Feb 10 2023, 5:01 AM
mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

We now have emitSilenceableError() << "message", no need to do the creation dance manually here.

qcolombet added inline comments.Feb 10 2023, 5:12 AM
mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

Ah cool!
Let me adopt that.

qcolombet updated this revision to Diff 496443.Feb 10 2023, 5:47 AM

Adopt emitSilenceableFailure

mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

Don't hesitate to clean a few other things as you go: usage has not been consistently improved as you can see ..

Thank you!

qcolombet updated this revision to Diff 496444.Feb 10 2023, 5:50 AM

Update the added test check-lines.

qcolombet marked an inline comment as done.Feb 10 2023, 8:15 AM
qcolombet added inline comments.
mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

@ftynse @nicolasvasilache
Should we introduce a emitSilenceableRemark?

Scanning through the code, it looks like we still use the old style in a few places, and sometimes we emit Remark not Error and I don't know if we want to promote these to errors.
E.g.,
https://github.com/llvm/llvm-project/blob/af39acda8873cc75db116e326588447f018a99d9/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp#L668

ftynse added inline comments.Feb 10 2023, 8:59 AM
mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

Places that emit Remark are wrong and should emit Error instead.

qcolombet marked an inline comment as done.Feb 13 2023, 2:02 AM
qcolombet added inline comments.
mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp
27–28

Changed that in https://github.com/llvm/llvm-project.git

2a58be423963..5a028cc8ffa5  main -> main
ftynse accepted this revision.Feb 13 2023, 2:03 AM
This revision is now accepted and ready to land.Feb 13 2023, 2:03 AM
This revision was landed with ongoing or failed builds.Feb 13 2023, 3:35 AM
This revision was automatically updated to reflect the committed changes.
qcolombet marked an inline comment as done.