[PATCH] D33734: [AArch64] Add fallback in FastISel fp16 conversions

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added a reviewer: SjoerdMeijer.
kristof.beyls added a subscriber: SjoerdMeijer.
kristof.beyls added inline comments.


================
Comment at: lib/Target/AArch64/AArch64FastISel.cpp:2857-2860
+  // Let regular ISEL handle FP16
+  if (DestVT == MVT::f16)
+    return false;
+
----------------
It looks a bit strange to me that for selectFPToInt, the test is (SrcVT == MVT::f128 || SrcVT == MVT::f16) to fall back to DAGISel, while for selectIntToFP, the test is (DestVT == MVT::f16), not checking for a 128 bit floating point data type.
I guess this is because i128 is not a legal type for AArch64?

If my guess is correct, this change looks reasonable; otherwise I'm not entirely sure.


================
Comment at: test/CodeGen/AArch64/arm64-fast-isel-conversion-fallback.ll:1
+; RUN: llc -O0 -verify-machineinstrs -mtriple=arm64-eabi -mcpu=cortex-a57 < %s | FileCheck --enable-var-scope %s
+
----------------
I guess that -mcpu=cortex-a57 isn't needed in this test line?
For the actual tests, I think @SjoerdMeijer probably will be able to review them better than I can.


https://reviews.llvm.org/D33734