[PATCH] D143711: [ARM] Fix Crash in 't'/'w' handling without fp16/fp16

[Sam Elliott via Phabricator via llvm-commits]

lenary added inline comments.


================
Comment at: llvm/test/CodeGen/ARM/inlineasm-fp-half.ll:9
 
-define arm_aapcscc half @f_t(half %x) nounwind {
-; CHECK-LABEL: f_t:
-; CHECK:       @ %bb.0: @ %entry
-; CHECK-NEXT:    vmov.f16 s0, r0
-; CHECK-NEXT:    @APP
-; CHECK-NEXT:    vsqrt.f16 s0, s0
-; CHECK-NEXT:    @NO_APP
-; CHECK-NEXT:    vmov r0, s0
-; CHECK-NEXT:    bx lr
+; With FP16/BF16
+; RUN: llc -mtriple=arm-none-eabi -mattr=+armv8.2-a,+fp-armv8,+fullfp16,+bf16,-neon %s -o - | FileCheck %s --check-prefixes=FP16-SOFTFP
----------------
olista01 wrote:
> I think we should also have tests with FP16 but not BF16, because there are separate code paths for them.
So, I looked at doing this, and there is no difference in codegen between `+fullfp16,-bf16` and `+fullfp16,+bf16`. I believe this is because this codepath ends up relying on `MoveToHPR` and `MoveFromHPR`, which are only called often if the VT is `MVT::f16` or `MVT::bf16`, but the relevant code path inside the function only checks for `Subtarget->hasFullFP16()`. I'm not convinced this is incorrect, but I'm not sure. I think I need to do more investigation before I canonically say either way.

This might mean that the register class allocation for bf16 becomes entirely based on whether we have fullfp16, I'm not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143711