This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Affine] Add pass options to supervectorizer
ClosedPublic

Authored by jsetoain on Sep 26 2022, 2:54 AM.

Details

Summary

The only current options to create a supervectorization pass from an
external dialect is to use createSuperVectorizePass with the virtual
vector dimensions as a parameter, but the pass accepts other parameters.

This patch makes use of the autogenerated pass creation options to enable
pass configuration through AffineVectorizeOptions.

Diff Detail

Event Timeline

jsetoain created this revision.Sep 26 2022, 2:54 AM
jsetoain requested review of this revision.Sep 26 2022, 2:54 AM
jsetoain added inline comments.Sep 26 2022, 2:59 AM
mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
1720–1727

Note to reviewers: While working on this patch I noticed this "duplicated" code up here. It doesn't seem to be doing anything, and I've checked it can be safely removed. I really don't understand why it was left here, if it was an accident or something I'm missing, but I wanted to point it out. If it's unnecessary, we might as well get rid of it with this patch.

jsetoain updated this revision to Diff 462853.Sep 26 2022, 3:41 AM

clang-format

dcaballe accepted this revision.Sep 26 2022, 11:44 PM

Thanks, Javier! LGTM

mlir/lib/Dialect/Affine/Transforms/SuperVectorize.cpp
1720–1727

It looks like it was accidentally left there.

This revision is now accepted and ready to land.Sep 26 2022, 11:44 PM

Pass generation was updated such that this should already be exposed via AffineVectorizeOptions. See https://reviews.llvm.org/D134607 as an example for updating to enable auto generation of the create methods.

jsetoain updated this revision to Diff 463243.Sep 27 2022, 8:39 AM

Move on to the new autogenerated pass builders

jsetoain edited the summary of this revision. (Show Details)Sep 27 2022, 8:45 AM

Pass generation was updated such that this should already be exposed via AffineVectorizeOptions. See https://reviews.llvm.org/D134607 as an example for updating to enable auto generation of the create methods.

Ah! I somehow missed this. Thanks for pointer, River! It looks much better this way. On the flip side, this might break things downstream if somebody was using the old method with the "virtualVectorSize" parameter. I think this is cleaner and the potential minor issue acceptable, but if it's not, I'm happy to re-introduce it for "backwards compatibility". In any case, I will let it sit here for a few days so people can see it coming.

jsetoain marked an inline comment as done.Sep 27 2022, 9:01 AM
dcaballe accepted this revision.Sep 27 2022, 10:41 AM

Awesome! I was not aware of that either. Thanks, River!
Yeah, this pass is pretty old. I wouldn't be surprised if more opportunities like this one come up.

On the flip side, this might break things downstream if somebody was using the old method with the "virtualVectorSize" parameter.

That sounds like a simple update. I would go with it.

This revision was automatically updated to reflect the committed changes.