Page MenuHomePhabricator

[ARM] Remove some spurious MVE reduction instructions.

Authored by simon_tatham on Thu, Sep 5, 1:28 AM.



The family of 'dual-accumulating' vector multiply-add instructions
(VMLADAV, VMLALDAV and VRMLALDAVH) can all operate on both signed and
unsigned integer types, and they all have an 'exchange' variant (with
an X in the name) that modifies which pairs of vector lanes in the two
inputs are multiplied together. But there's a clause in the spec that
says that the X variants don't operate on unsigned integer types,
only signed. You can have X, or unsigned, or neither, but not both.

We didn't notice that clause when we implemented the MC support for
these instructions, so LLVM believes that things like VMLADAVX.U8 do
exist, contradicting the spec. Here I fix that by conditioning them
out in Tablegen.

In order to do that, I've reversed the nesting order of the Tablegen
multiclasses for those instructions. Previously, the innermost
multiclass generated the X and not-X variants, and the one outside
that generated the A and not-A variants. Now X is done by the outer
multiclass, which allows me to bypass that one when I only want the
two not-X variants.

Changing the multiclass nesting order also changes the names of the
instruction ids unless I make a special effort not to. I decided that
while I was changing them anyway I'd make them look nicer; so now the
instructions have names like MVE_VMLADAVs32 or MVE_VMLADAVaxs32,
instead of cumbersome _noacc_noexch suffixes.

The corresponding multiply-subtract instructions are unaffected. Those
don't accept unsigned types at all, either in the spec or in LLVM.

Diff Detail


Event Timeline

simon_tatham created this revision.Thu, Sep 5, 1:28 AM

Change looks sensible to me, but how difficult would this be to do without the, umm, creative use of foreach?

This revision is now accepted and ready to land.Mon, Sep 9, 1:20 AM

Reworked the patch to reverse the multiclass nesting order, avoiding the need to fake a Tablegen if using foreach.

simon_tatham edited the summary of this revision. (Show Details)Mon, Sep 9, 7:25 AM
dmgreen accepted this revision.EditedMon, Sep 9, 8:12 AM

Nice one. LGTM.

This revision was automatically updated to reflect the committed changes.