[PATCH] D81373: [WIP] Basic bfloat support on Arm

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea marked an inline comment as done.
labrinea added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5908
+    if ((DstVT == MVT::f16 && !Subtarget->hasFullFP16()) ||
+        (DstVT == MVT::bf16 && !Subtarget->hasBF16()))
       return SDValue();
----------------
dmgreen wrote:
> According to D81411, you can have bf16 without having fp16. And so you don't have any of the instructions like VMOV.f16 (which a VMOVrh will turn into).
> 
> Same goes for the vldr.16 int he test below. Because +fp16 isn't specified, we might have to awkwardly use some other set of instructions. It will be more efficient to use vmov.16 and vldr.16 if they are available, but if they are not we might have to fall back to something else.
> 
> Or we say that combination isn't supported, but it seems that fp16 is still optional and bf16 is mandatory in 8.6.
Good point. I am going to alter these checks to only guard fullfp16 for now. As the title suggests this is basic support, so I think it's fair to only support the bf16+fullfp16 combination in this revision. I will make sure it is explicitly stated in the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81373