Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[mlir][transform] Add support for expressing scalable vector sizes

Authored by awarzynski on Jun 1 2023, 8:15 AM.



This patch enables specifying scalable vector sizes when using the
Transform dialect to drive vectorisation, e.g.:

transform.structured.masked_vectorize %0 vector_sizes [8, 16, [4]]

This is implemented by extending the MaskedVectorizeOp with a dedicated
attribute for "scalability" and by creating a custom parser. At the
moment, only the trailing vec size can be scalable. The following is not
yet supported:

transform.structured.masked_vectorize %0 vector_sizes [8, [16], [4]]

As the vectoriser does not support scalable vectorisation just yet, it's
tricky to test this change. In the meantime, use the debug output,
--debug-only=linalg-vectorization, to check whether scalable
vectorisation has been switched on.

This change is a part of a larger effort to enable scalable
vectorisation in Linalg. See this RFC for more context:

Similar patch for tiling:

Diff Detail

Event Timeline

awarzynski created this revision.Jun 1 2023, 8:15 AM
Herald added a project: Restricted Project. · View Herald Transcript
awarzynski requested review of this revision.Jun 1 2023, 8:15 AM

Thanks! It looks like a good starting point to me. I should be able to add support to the vectorizer with this and the proper public API. Leaving the TD approval to Alex or Nicolas. I'm ok with landing this in the meantime.

ftynse requested changes to this revision.Jun 2 2023, 1:30 AM

Can we please have a parser roundtrip test for the changes? Printer and parser don't seem to match.


Nit: don't hardcode 4 here.


Why is this necessary?


Why can't this just parseType?


Shouldn't this be a property?


I think this needs to account for isLastVectorSizeScalable.


This should also print the attribute dictionary.


If you are looking for a way to test this before it's fully supported, how about op->emitWarning("scalable vectorization not supported yet") that can be checked with --verify-diagnostics?

This revision now requires changes to proceed.Jun 2 2023, 1:30 AM
ftynse added inline comments.Jun 2 2023, 1:31 AM

Would it be possible to use the custom directive for this, additionally taking the "scalable" attribute? The code looks long and boilerplaty.

Matt added a subscriber: Matt.Jun 3 2023, 11:16 AM
awarzynski added inline comments.Jun 7 2023, 9:40 AM

Sure, thanks for the suggestion! I will upload a new version shortly.


Thanks, that's a great suggestion!

awarzynski updated this revision to Diff 529351.Jun 7 2023, 9:49 AM

Switch back to using custom directive, add a test.

I was able to get rid of the custom printer/parser, but had to tweak a few other things. Nothing major.

ftynse accepted this revision.Jun 8 2023, 3:36 AM
This revision is now accepted and ready to land.Jun 8 2023, 3:36 AM