[PATCH] D58813: [ARM] Fix select_cc lowering for fp16

Tue Mar 5 00:45:48 PST 2019

SjoerdMeijer accepted this revision.
SjoerdMeijer added inline comments.
This revision is now accepted and ready to land.

================
Comment at: test/CodeGen/ARM/vsel-fp16.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=armv8a--none-eabi -mattr=+fp-armv8,+fp16,+fullfp16 -float-abi=hard | FileCheck %s
+
----------------
olista01 wrote:
> SjoerdMeijer wrote:
> > Just for simplicity, can we omit +fp16 here? Or do we actually need it?
> > 
> > For a moment I was wondering if we also need to test this with -fullfp16? I appreciate we are not testing this change anymore, but then remembered we are doing that in fp16-instructions.ll, which also contains some VSEL checks. Perhaps we can move these tests to there, or vice versa, to keep the VSEL tests together?
> I think I can remove both +fp-armv8 and +fp16.
> 
> I don't think we need to test with -fullfp16: in that case f16 isn't a legal type in the backend, so these code paths won't get hit. I copied the tests from vsel.ll and changed the types, because that looks like a comprehensive set of tests for the different condition codes.
sounds good, lgtm.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58813