This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Selection for MVE VMOVN
ClosedPublic

Authored by dmgreen on Oct 1 2019, 10:24 AM.

Details

Summary

The adds both VMOVNt and VMOVNb instruction selection from the appropriate shuffles. We detect shuffle masks of the form:
0, N, 2, N+2, 4, N+4, ...
or
0, N+1, 2, N+3, 4, N+5, ...
ISel will also try the opposite patterns, with inputs reversed. These are selected to VMOVNt and VMOVNb respectively.

Diff Detail

Event Timeline

dmgreen created this revision.Oct 1 2019, 10:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2019, 10:24 AM
samparker added inline comments.Oct 2 2019, 7:39 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
6918

Why the checks for M[i]>= 0?

dmgreen marked an inline comment as done.Oct 2 2019, 7:56 AM

There might be something wrong with this. A test I just ran failed in a way I don't understand yet. Give me a minute to figure out what's up.

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

M[i] == -1 is "undef", so we can put whatever we want there.

False alarm. I think the script just hickuped on the build for some reason. A rerun did just fine on the same thing.

samparker added inline comments.Oct 3 2019, 3:24 AM
llvm/lib/Target/ARM/ARMISelLowering.cpp
6918

Ah, ok, cheers.

6936

How about refactoring these two functions? The +1 is the only difference, right?

dmgreen updated this revision to Diff 223025.Oct 3 2019, 8:20 AM

Now as one function

samparker accepted this revision.Oct 3 2019, 8:46 AM

Cool. LGTM

This revision is now accepted and ready to land.Oct 3 2019, 8:46 AM
dmgreen updated this revision to Diff 224000.Oct 9 2019, 1:34 AM

It turns out that this as incorrrect, just not for the same reason I was thinking. I was missing type checks, so we were trying to form VMOVN's for 32bit types (which would fail to select). I've added the type checks and some extra tests for such cases.

This revision was automatically updated to reflect the committed changes.