Page MenuHomePhabricator

[ARM] MVE Vector Shifts
ClosedPublic

Authored by dmgreen on Jul 4 2019, 9:19 AM.

Details

Summary

This is basic shifts for MVE. There are many shifts in MVE, but the ones handled here are:

  • VSHL (imm)
  • VSHRu (imm)
  • VSHRs (imm)
  • VSHL (vector)
  • VSHL (register)

MVE, like NEON before it, doesn't have shift right by a vector (or register). We instead have to negate the amount and shift in the opposite direction. This means we have to convert any SHR's into a from of SHL (that is still signed or unsigned) with a negated condition and selecting from there. MVE still does have shifting by an immediate for SHL, ASR and LSR.

I've added lowering for these and for register forms, which work well for shift lefts but may require an extra fold of neg(vdup(x)) -> vdup(neg(x)) to optimally for right shifts.

Diff Detail

Repository
rL LLVM

Event Timeline

dmgreen created this revision.Jul 4 2019, 9:19 AM

Just a general question first, I was wondering about this:

NEON was previously lowering the shifts into intrinsics and selecting on the intrinsic. I've converted that to a node shared between NEON and MVE which shifts left whilst still storing sign (ARMISD::VSHLs and ARMISD:VSHLu)

Would it perhaps be useful to do this in 2 steps? First the refactoring, the NEON part and the introduction of the nodes, which should be a NFC, and then in a 2nd follow up patch the MVE changes?

Sure, I can do that.

Cheers, makes the patches smaller and easier to review too :-)

dmgreen updated this revision to Diff 208737.Jul 9 2019, 10:35 AM
dmgreen edited the summary of this revision. (Show Details)

Now sits on top of D64426, this just handles the MVE side of things.

SjoerdMeijer added inline comments.Jul 10 2019, 6:24 AM
llvm/test/CodeGen/Thumb2/mve-shifts.ll
109 ↗(On Diff #208737)

Function 'isVShiftLImm' is checking that all constants are the same. Do we have already a test somewhere that checks what happens if e.g. one of the constants is different?

dmgreen updated this revision to Diff 209781.Jul 15 2019, 2:11 AM

Added extra tests

This revision is now accepted and ready to land.Jul 15 2019, 2:17 AM
Closed by commit rL366056: [ARM] MVE Vector Shifts (authored by dmgreen, committed by ). · Explain WhyJul 15 2019, 4:35 AM
This revision was automatically updated to reflect the committed changes.