This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add vrev32 NEON fp16 patterns
ClosedPublic

Authored by dmgreen on Oct 28 2019, 5:54 AM.

Details

Summary

These Neon patterns for vrev32.16 appear to be missing, only the i16 patterns are present.

Diff Detail

Event Timeline

dmgreen created this revision.Oct 28 2019, 5:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2019, 5:54 AM

These Neon patterns for vrev32.16 appear to be missing, only the i16 patterns are present.

Hmm, that's kind of what I was expecting: support for i16s, but nog f16s because that not only requires HasNeon but also HasFullFP16. Without HasFullFP16, are vectors of f16s legal?

Yep! That's what I meant. Neon+fullfp16, this fails to select. This test runs in that configuration now. The RHS check lines are missing because it crashes failing to select.

Without fullfp16 you are right that we would never see a f16 vector, so the patterns would never be used. We would promote the f16's to f32's before that point, I believe.

Yep! That's what I meant. Neon+fullfp16, this fails to select. This test runs in that configuration now. The RHS check lines are missing because it crashes failing to select.

Without fullfp16 you are right that we would never see a f16 vector, so the patterns would never be used. We would promote the f16's to f32's before that point, I believe.

Okidoki, cheers, that makes perfect sense.
Does that mean we only need to add HasFullFP16 to the Predicates of the patterns?

I think not, because the instruction it is actually selecting is just a vrev32, available whenever Neon is present. They will just never be used when fullfp16 is not present. Same as the existing patterns on line 6800 (which I just formatted here).

SjoerdMeijer accepted this revision.Oct 29 2019, 2:17 AM

Yep, cheers, LGTM

This revision is now accepted and ready to land.Oct 29 2019, 2:17 AM
This revision was automatically updated to reflect the committed changes.