This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Lower v16i8 -> i64 VMLA reductions.
ClosedPublic

Authored by dmgreen on Jul 9 2021, 1:02 AM.

Details

Summary

MVE does not have a VMLALV instruction that can perform v16i8 -> i64 reductions, like it does for v8i16->i64 and v4i32->i64 reductions. That means that the pattern to create them will be spilt up by type legalization, creating a lot of instructions.

This extends the patterns for matching i64 reductions a little to handle the v16i8->i64 case. We need to turn them into a pair of v8i16->i64 VMLALVs that each perform half of the reduction and are summed together (so the later is a VMLALVA). The order of the lanes does not matter for the reduction so we generate a MVEEXT for the extension, that will either be folded into a extending load or can be optimized to a VREV/VMOVL. Some of the resulting codegen isn't optimal, but will be improved in a later patch.

Diff Detail

Event Timeline

dmgreen created this revision.Jul 9 2021, 1:02 AM
dmgreen requested review of this revision.Jul 9 2021, 1:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 9 2021, 1:02 AM
SjoerdMeijer added inline comments.Jul 9 2021, 2:21 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
16051

Perhaps a silly question on the use of 'illegal'. Type v16i8 isn't an illegal type, it is just not supported for these VMLALV instructions. Thus, I was wondering if 'unsupported' would be better, to avoid possible confusion with type legalization in general if that makes sense?

dmgreen added inline comments.Jul 9 2021, 4:19 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
16051

I think both class as legalization. You both legalize the types and legalize the operations. A v4i32 bitreverse is illegal, for example, even if the v4i32 type is legal. We need to turn it into a v16i8 bitreverse and a VREV32.

I don't have a super strong opinion if you think it's best to change it - I think the two words essentially mean the same thing here :) But illegal sounds fine to me.

SjoerdMeijer accepted this revision.Jul 9 2021, 7:03 AM

LGTM

llvm/lib/Target/ARM/ARMISelLowering.cpp
16051

No, it's fine by me too, just started to wonder about it....

This revision is now accepted and ready to land.Jul 9 2021, 7:03 AM
This revision was landed with ongoing or failed builds.Jul 14 2021, 10:11 AM
This revision was automatically updated to reflect the committed changes.