This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Fixes for ARMv8.2-A FP16 scalar intrinsic
ClosedPublic

Authored by az on Feb 6 2018, 4:07 PM.

Details

Reviewers
SjoerdMeijer
Summary

A couple of fixes on top of https://reviews.llvm.org/D41792:

  • Fixes for freceprical, and fsqrt instructions in the backend.
  • The intrinsics that generate builtin calls with i16 data types fails in instruction selection. In a preparation for future fixes in the backend for these, the code generated by the frontend is modified. For example, a builtin that returns i16 is implemented by returning i32 that is truncated to i16. This is done similar to other non-fp16 intrinsics that produces i16 data types.
  • Fix frontend code generated for abs.

Diff Detail

Event Timeline

az created this revision.Feb 6 2018, 4:07 PM

Thanks for fixing this. Looks very reasonable to me.

Question about the failures: I am now wondering if this means we were and still
are missing tests?

Nit: for future reviews, I think it is better to split patches up if they are commits to
different repos.

az updated this revision to Diff 133505.Feb 8 2018, 3:40 PM

Question about the failures: I am now wondering if this means we were and still are missing tests?

Given that this work is fixing D41792 which is mainly about adding frontend intrinsic support, then there is a test for each intrinsic and I am updating the tests for the intrinsics with code generation changes (they only test the frontend). As we previously discussed it, there is some work to be done in instruction selection in order to generate good IR for some intrinisics and the ones with problems need new tests. In this patch, I am fixing few simple ones that fall into that category (see AArch64InstrInfo.td) and I agree that I should have separated this patch into clang and llvm patches instead of just one. I am adding tests for those in this revision.

This revision is now accepted and ready to land.Feb 9 2018, 12:26 AM
az closed this revision.Feb 12 2018, 2:08 PM

Committed as r324940 and r324912