Page MenuHomePhabricator

Teach the AArch64 backend about v4f16 and v8f16
ClosedPublic

Authored by olista01 on Aug 18 2014, 7:04 AM.

Details

Summary

This is the vector counterpart to D4879, and teaches the AArch64 backend to deal with the operations required to deal with the operations on v4f16 and v8f16 which are exposed by NEON intrinsics, plus the add, sub, mul and div operations.

Diff Detail

Event Timeline

olista01 updated this revision to Diff 12615.Aug 18 2014, 7:04 AM
olista01 retitled this revision from to Teach the AArch64 backend about v4f16 and v8f16.
olista01 updated this object.
olista01 edited the test plan for this revision. (Show Details)
olista01 added a reviewer: t.p.northover.
olista01 set the repository for this revision to rL LLVM.
olista01 added a subscriber: Unknown Object (MLST).
t.p.northover edited edge metadata.Aug 18 2014, 8:10 AM

Hi Oliver,

It looks like there are various components to this patch, some without tests. And some of the tests that are here are a bit generic (it'd be nice to track that the correct operands get used, rather than just that an "fdiv" is emitted, for example).

  • There appear to be no tests for the ABI changes.
  • Hardly any coverage of load/store patterns or the C++ ISel
  • bitcast also seems to get short shrift, given how substantial the code changes are (mostly just used when it has to be as part of other shuffle testing).

We don't necessarily need blanket coverage (v4f16 is fundamentally very similar to v4i16) but something in between would be good.

Cheers.

Tim.

lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
385–388

This renders the comment just above wrong. Could you fix that?

olista01 updated this revision to Diff 12700.Aug 20 2014, 8:15 AM
olista01 edited edge metadata.
  • Added ABI tests
  • Added tests for NEON loads/stores (and added a few missing patterns)
  • Added tests for bitcasts
  • Tightened up tests which do not specify registers (ignoring some of the larger v8f16 outputs, as they use many vector inserts/extracts and re-use registers which I could not work out a good way to test)
  • Fixed out of date comment
rengolin accepted this revision.Aug 27 2014, 8:59 AM
rengolin added a reviewer: rengolin.
rengolin added a subscriber: rengolin.

Hi Oliver,

Tim is on holidays...

Given that I didn't find anything wrong with the patch and that Tim's only complaints were the lack of testing (and that you have added a lot more and fixed the ones he complained about), I'm going to approve this patch.

I'll also send an email to Tim, so if he has any additional concerns, he can do a post-commit review.

Thanks,
--renato

This revision is now accepted and ready to land.Aug 27 2014, 8:59 AM
olista01 closed this revision.Aug 27 2014, 9:25 AM

Thanks, committed revision 216555.