This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Make v2i1 legal
ClosedPublic

Authored by dmgreen on Nov 23 2021, 9:06 AM.

Details

Summary

MVE can treat v16i1, v8i1, v4i1 and v2i1 as different views onto the same 16bit VPR.P0 register, with v2i1 holding two 8 bit values for the two halves. This was never treated as a legal type in llvm in the past as there are not many 64bit instructions and no 64bit compares. There are a few instructions that could use it though, notably a VSELECT (as it can handle any size using the underlying v16i8 VPSEL), AND/OR/XOR for similar reasons, some gathers/scatter and long multiplies and VCTP64 instructions.

This patch goes through and makes v2i1 a legal type, handling all the cases that fall out of that. It also makes VSELECT legal for v2i64 as a side benefit. A lot of the codegen changes as a result - usually in way that is a little better or a little worse, but still expensive. Costs can change a little too in the process, again in a way that expensive things remain expensive. A lot of the tests that changed are mainly to ensure correctness - the code can hopefully be improved in the future where it comes up in practice.

The intrinsics currently remain using the v4i1 they previously did to emulate a v2i1. This will be changed in a followup patch but this one was already large enough.

Diff Detail

Event Timeline

dmgreen created this revision.Nov 23 2021, 9:06 AM
dmgreen requested review of this revision.Nov 23 2021, 9:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2021, 9:06 AM

LGTM with a couple of changes

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

How comes this is f64 and not i64? Is i64 not a valid element type for this?

8448–8455

It would be useful to update the comment here to include v2i1

8955–8963

Same here

dmgreen added inline comments.Nov 26 2021, 5:52 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
8366

It was originally because a f64 lane extract (into a D register) can be simpler than using a i64 register (which is not going to be legal). So the f64 shuffles and whatnot would be simpler. I don't think that's necessary from the tests, but it sounds a little better to keep them as legal types.

dmgreen updated this revision to Diff 390035.Nov 26 2021, 5:54 AM

Add some extra comments.

samtebbs accepted this revision.Nov 26 2021, 7:06 AM

Great, LGTM

This revision is now accepted and ready to land.Nov 26 2021, 7:06 AM
This revision was landed with ongoing or failed builds.Dec 3 2021, 6:05 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/CodeGen/Thumb2/mve-vctp.ll