[PATCH] D94557: [ARM] Fixed incorrect lowering when using GNUEABI (libgcc) and 16bit floats

[Kevin Peizner via Phabricator via llvm-commits]

kevinpeizner added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:720
       { RTLIB::FPROUND_F32_F16, "__aeabi_f2h", CallingConv::ARM_AAPCS },
       { RTLIB::FPROUND_F64_F16, "__aeabi_d2h", CallingConv::ARM_AAPCS },
       { RTLIB::FPEXT_F16_F32, "__aeabi_h2f", CallingConv::ARM_AAPCS },
----------------
Here `__gnu_d2h_ieee` is replaced with `__aeabi_d2h` if and only if EABI is targeted.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:601
       { RTLIB::FPROUND_F64_F32, "__aeabi_d2f", CallingConv::ARM_AAPCS, ISD::SETCC_INVALID },
-      { RTLIB::FPROUND_F64_F16, "__aeabi_d2h", CallingConv::ARM_AAPCS, ISD::SETCC_INVALID },
+      { RTLIB::FPROUND_F64_F16, "__gnu_d2h_ieee", CallingConv::ARM_AAPCS, ISD::SETCC_INVALID },
       { RTLIB::FPEXT_F32_F64,   "__aeabi_f2d", CallingConv::ARM_AAPCS, ISD::SETCC_INVALID },
----------------
rengolin wrote:
> This doesn't look like the right fix. You're just replacing for all, no? Your changes to the tests don't show it can still emit EABI intrinsics with the right ABI flag.
The `i16 @test_to_fp16(double %in)` in `llvm/test/CodeGen/ARM/fp16.ll` test checks for different intrinsics based on the EABI flag. For example, when the test runs with 
`; RUN: llc -mtriple=armv7a--none-eabi < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-HARDFLOAT-EABI %s` then the test will verify that `bl __aeabi_d2h` is emitted rather than `bl __gnu_d2h_ieee`.

I interpret this to mean that my changes still emit EABI intrinsics with the right ABI flag. This is my first contribution to the LLVM Project so please let me know if this is incorrect.

The intent is to emit `__gnu_d2h_ieee` by default while allowing `__aeabi_d2h` to be emitted when EABI is targeted.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94557/new/

https://reviews.llvm.org/D94557