[PATCH] D38315: [ARM] Add f16 type support and code generation (part 1/2)

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: test/CodeGen/ARM/fp16-instructions.ll:14
+;CHECK-FP16-LABEL:      Sub:
+;CHECK-FP16:            vsub.f32  s0, s0, s2
+;CHECK-FP16-NEXT:       mov pc, lr
----------------
olista01 wrote:
> This looks like it's returning the result as a 32-bit float, which is wrong. It should be a 16-bit float in the least-significant half of s0.
> 
> Also, are there any conversion instructions before the vsub? If so, it would be better to include them in the test, and if not then the arguments are being passed incorrectly too.
Thanks Oliver. You're right and I do need to change the implementation of the calling conventions. For return values, the aapcs says:
"A Half-precision Floating Point Type is returned in the least significant 16 bits of r0", so yes we want to see an vcvtb.f16.f32 s0, s0 for the return value here.

And there are converts for the arguments at the moment, in fact 2 for each arguments. It thinks the args are passed in f32 regs. So there is a convert to f16, but then  there's another convert to f32 because the operations is done in f32 (in this case).  The aapcs says that:
"If the argument is a Half-precision Floating Point Type its size is set to 4 bytes as if it had been copied to the least significant bits of a 32-bit
register and the remaining bits filled with unspecified values", and so yes the args passing needs changing too.


https://reviews.llvm.org/D38315