[PATCH] D82372: [ARM][BFloat] Legalize bf16 type even without fullfp16.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added a comment.

Hi Simon, thanks for working on this. Looks good overall. A few remarks inline.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:726
 
-    // For the time being bfloat is only supported when fullfp16 is present.
-    if (Subtarget->hasBF16())
-      addRegisterClass(MVT::bf16, &ARM::HPRRegClass);
+  addRegisterClass(MVT::bf16, &ARM::HPRRegClass);
+  if (!Subtarget->hasFullFP16()) {
----------------
Shouldn't this and the following 4 lines be contitional to `Subtarget->hasBF16()` ?


================
Comment at: llvm/lib/Target/ARM/ARMInstrVFP.td:171
+                    (COPY_TO_REGCLASS (LDRH addrmode3:$addr), HPR)> {
+    let Predicates = [HasNoFPRegs16];
+  }
----------------
Can you either define a new pattern with this predicate, or delete the `FPRegs16Pat` pattern and use the same syntax for uniformity? Just a personal preference.


================
Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:595
       }
+    } else if (AddrMode == ARMII::AddrMode3) {
+      // Sign-extending loads.
----------------
Why do we need this block of changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82372