[PATCH] D81373: [ARM] Basic bfloat support

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea marked 4 inline comments as done.
labrinea added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:727
+  if (Subtarget->hasBF16())
+    addRegisterClass(MVT::bf16, &ARM::HPRRegClass);
+
----------------
dmgreen wrote:
> We only add the FP16 regclass if we have HasFullFP16. Not if we just have the vcvt instructions (HasFP16). I agree it is probably good to make bf16 a legal type if we can, but a lot of operations will not be supported. As far as I understand they are not expected to work, and there will be nothing that can promote them at the moment?
> 
> I'm not sure how the AArch64 backend handled it, but do we need something like setAllExpand(..)? Honestly I would expect it to still break if you tried to add two bfloats in IR, but it might be more future-proof.
Maybe for now it'd be better to add the bfloat type if both fullfp16 and bf16 are present. Makes sense?


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:5081
 
 bool ARMTargetLowering::isUnsupportedFloatingType(EVT VT) const {
   if (VT == MVT::f32)
----------------
dmgreen wrote:
> This controls some things like setcc lowering. I wouldn't expect them to be relevant for bfloat if it is essentially just a storage type.
fair enough, I'll remove it


================
Comment at: llvm/lib/Target/ARM/ARMInstrVFP.td:166
 
+def : Pat<(f16 (alignedload16 addrmode5fp16:$addr)),
+          (VLDRH addrmode5fp16:$addr)>;
----------------
dmgreen wrote:
> I think these patterns should still have `let Predicates = [HasFPRegs16] in` around them, like we do for all the NEON or MVE patterns.
I'll create a predicated pattern


================
Comment at: llvm/test/CodeGen/ARM/bfloat.ll:9
+define bfloat @load_scalar_bf(bfloat* %addr) {
+; CHECK-LABEL: load_scalar_bf:
+; CHECK:       @ %bb.0: @ %entry
----------------
dmgreen wrote:
> These CHECK lines are left over.
Oops, didn't notice. I'll remove them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81373