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.
Details
Diff Detail
Event Timeline
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 | ||
---|---|---|
387–394 | This renders the comment just above wrong. Could you fix that? |
- 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
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 renders the comment just above wrong. Could you fix that?