This patch simply adds vectorize_nd_extract (that's currently used for
the vectorize Op) to masked_vectorize. A test is added to verify
that it works as expected - it prevents the masked vectorisation of
tensor.extract, which is currently not supported.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@dcaballe Is this what you had in mind? The change is rather trivial, but exposes a small design/implementation mismatch between transform.structured.vectorize and transform.structured.masked_vectorize. Note how:
- the former goes via applyToOne, which does not issue any diagnostics,
- the latter implements apply, which does issue any diagnostics.
This is significant in this case, because a diagnostic leads to a non-zero return code from the driver (e.g. mlir-opt), which means that e.g. "vectorization.mlir" fails (non-zero return code means is interpreted as a test failure by LIT). Perhaps my update is against what's intended here, but then we should look for some other way to make sure that we get consistent behavior across Transfer dialect Ops (at least in the context of the Linalg vectorizer).
transform.vectorize is a blanket op that applies a bunch of patterns greedily, it is unfortunate but it is what gets us an unsurprising behavior for now, it will be improved over time.
transform.masked_vectorize is targeted to 1 op because it needs the vector_sizes information which comes from the user injecting static information.
I am not clear what this PR aims to achieve.
If you wanted to just turn off the silenceable error that vectorization emits, you can simply wrap the transform into a
transform.sequence failures(suppress) {
instead of a
transform.sequence failures(propagate) {
Thanks for taking a look!
This is exactly what I was after. Let me removed all other changes that are not needed and update the summary.
Replace failures(propagate) with failures(suppress), remove unrelated changes that are not needed.
If you wanted to just turn off the silenceable error that vectorization emits, you can simply wrap the transform into a
We can also check for emitted diagnostics using -verify-diagnostics if we expect the transform op to fail, silencing failures in tests isn't necessarily the best idea. We could also split out this test into a separate file instead of appending to a 2k line test that becomes unmanageable.
Incorporate suggestions from @ftynse
- Moved the test to a newly created file: vectorization-unsupported.mlir
- Added --verify-diagnostics