This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Re-define masking semantics in vector.transfer ops
ClosedPublic

Authored by dcaballe on Nov 15 2022, 5:03 PM.

Details

Summary

Masking hasn't been widely used in vector transfer ops and the semantics
of the mask operand were a bit loose. This patch states that the mask
operand in a vector transfer op is applied to the read/write part of the
operation and, therefore, its shape should match the shape of the
elements read/written from/into the memref/tensor regardless of any
permutation/broadcasting also applied by the transfer operation.

Diff Detail

Event Timeline

dcaballe created this revision.Nov 15 2022, 5:03 PM
Herald added a project: Restricted Project. · View Herald Transcript
dcaballe requested review of this revision.Nov 15 2022, 5:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2022, 5:03 PM

Nice, thanks for tightening this up @dcaballe !

Now that you are looking more seriously at this, do you have a recommendation on whether the op should be simplified (or even split into multiple ops) ?
I am thinking in particular about the interaction between the required scalar padding value, inbounds attribute and the mask.

Thoughts?

This revision is now accepted and ready to land.Nov 16 2022, 11:51 AM

Now that you are looking more seriously at this, do you have a recommendation on whether the op should be simplified (or even split into multiple ops) ?
I am thinking in particular about the interaction between the required scalar padding value, inbounds attribute and the mask.

Yes, definitely. I think at this point vector transfer ops are conflating far too many things and the complexity outweighs the benefits. The most challenging part, imo, is the permutation map as we are mixing memory ops with "in-register" data manipulation. It gets even worse with masking as the masking action happens only on the memory part of the transfer op, so there is a little dance of affine maps needed to actually know what we really have to mask. As a first step, I think we should decouple data manipulation (permutation map) from the load operation and generate independent transpose/broadcast ops. Regarding the inbounds attribute, it doesn't make sense for masked operations as the point of using a mask is to make the accesses always inbounds. The padding value should also be aligned with the padding value needed for masking.

I think the last two tasks are good starter tasks. Decoupling the permutation map part is going to be painful, though.

This revision was landed with ongoing or failed builds.Nov 17 2022, 5:07 PM
This revision was automatically updated to reflect the committed changes.