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

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
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()) {
----------------
labrinea wrote:
> Shouldn't this and the following 4 lines be contitional to `Subtarget->hasBF16()` ?
How did I miss //that?!// Thanks, good catch.


================
Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:595
       }
+    } else if (AddrMode == ARMII::AddrMode3) {
+      // Sign-extending loads.
----------------
labrinea wrote:
> Why do we need this block of changes?
I put it in because otherwise the Thumb invocations of `llc` in my new test file failed with the 'Unsupported addressing mode' assertion in this function.

However, now I look more closely, that's probably because I was using the wrong instruction – I was using `LDRH` for both Arm and Thumb, whereas I should have used `t2LDRHi12` in Thumb. With that change, I don't seem to need this extra addressing mode support any more.

(But I couldn't tell you //why// it's not needed, because as far as I can tell `LDRH` itself still does use addressing mode 3, and is still selected in Arm state.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82372