This is an archive of the discontinued LLVM Phabricator instance.

[ARM] MVE predicate register support
ClosedPublic

Authored by dmgreen on Jul 21 2019, 7:04 AM.

Details

Summary

This adds support code for building and shuffling i1 predicate registers. It generally uses two basic principles, either converting the predicate into an scalar (through a PREDICATE_CAST) and doing scalar operations on it there, or by converting the register to an full vector register and back.

Some of the code here is a not super efficient but will hopefully cover most cases of moving i1 vectors around and can be improved in subsequent patches.

Some code by David Sherwood.

Diff Detail

Event Timeline

dmgreen created this revision.Jul 21 2019, 7:04 AM

Just a first scan, I will look more closely tomorrow.

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

perhaps a message in this assert.

7385

is it weird to call createNEONModImm here?

7408

what do you mean with the "real predicate"?

7679

do we want to check or assert that we're targeting MVE here?

7787

Can this be a little helper function/lambda so that we don't duplicate this below for Op2?

7870

and this (plus the switch above) looks suspiciously similar, except that the node is a ISD::EXTRACT_VECTOR_ELT here.

dmgreen marked 6 inline comments as done.Jul 22 2019, 2:31 PM
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMISelLowering.cpp
7385

It was never renamed, and the same between Neon and MVE (it was used in the VMOVimm code too.)

I can rename it easily enough. createVModImm sound OK?

7408

The predicate may be a v4i1 or a v8i1. We cast it to a v16i1 to get the types right, using the principle that a v4i1 of abcd and a v16i1 of aaaabbbbccccdddd are the same thing.

dmgreen updated this revision to Diff 211193.Jul 22 2019, 2:32 PM
dmgreen marked 2 inline comments as done.
SjoerdMeijer added inline comments.Jul 22 2019, 11:53 PM
llvm/lib/Target/ARM/ARMISelLowering.cpp
7398

I can rename it easily enough. createVModImm sound OK?

Yep, sounds good

7408

Ah okay, cheers, got it.
Perhaps you just want to add this to the comments instead of the "If the real predicate is not v16i1 ".

7681

Perhaps better to hide magic constant 16 in a MVE subtarget function or something like that?

samparker added inline comments.Jul 23 2019, 3:54 AM
llvm/test/CodeGen/Thumb2/mve-pred-build-var.ll
189

Why aren't these just one: bfi r2, r0, #0, #16 or uxth r2, r0?

dmgreen marked 7 inline comments as done.Jul 23 2019, 4:09 AM
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMISelLowering.cpp
7398

It ended up as createVMOVModImm in rL366790.

7681

I changed this around a little to use the scalar size.

llvm/test/CodeGen/Thumb2/mve-pred-build-var.ll
189

Thats "FIXME: Handle splats of the same value better." Maybe just a conditional mov of -1 (or something)? It's on the list of things to do.

dmgreen updated this revision to Diff 211274.Jul 23 2019, 4:12 AM
dmgreen marked an inline comment as done.

Updates and rebase

SjoerdMeijer accepted this revision.Jul 23 2019, 5:16 AM

This all looks very reasonable to me. I am happy with this if @samparker is happy too.

This revision is now accepted and ready to land.Jul 23 2019, 5:16 AM
This revision was automatically updated to reflect the committed changes.