[llvm] [AArch64][SVE] Support lowering masked loads/stores of <4 x bf16> and <8 x bf16> (PR #208744)

Shanzhi Chen via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 15 04:37:06 PDT 2026


================

----------------
chenshanzhi wrote:

Hi. Thanks for review comments. This change is tested.

Actually this is a NFC in the latest version (9ff409755c75e674e13bccf73ce9a26e03b8363b) because the original `if (VT.isFloatingPoint())` is in `addTypeForFixedLengthSVE` where `VT.isFixedLengthVector()` is asserted to be true, which means only fixed length vectors of f16/bf16/f32/f64 could take this branch. The range could be checked in `include/llvm/CodeGen/GenVT.inc`
```c++
  FIRST_FP_FIXEDLEN_VECTOR_VALUETYPE = v1f16,
  LAST_FP_FIXEDLEN_VECTOR_VALUETYPE = v256f64,
```
And I did check that we don't have fixed length vectors of something like f128, f8 or other in this range in AArch64 currently.

Functionally, this branch does not apply to vectors of f16/bf16.

And previously vectors of bf16 are never passed to `addTypeForFixedLengthSVE` because `useSVEForFixedLengthVectorVT` always returns `false` for vectors of bf16 in `AArch64TargetLowering::AArch64TargetLowering`.
```c++
      for (MVT VT : MVT::fp_fixedlen_vector_valuetypes()) {
        if (useSVEForFixedLengthVectorVT(
                VT, /*OverrideNEON=*/!Subtarget->isNeonAvailable()))
          addTypeForFixedLengthSVE(VT);
      }
```
And as mentioned in the third point of the *Notes for review*, I found that if bf16 is added directly in the switch statement in `useSVEForFixedLengthVectorVT`, there would be many test failures. Those are caused by passing vectors of bf16 to `addTypeForFixedLengthSVE` and taking this branch.
```c++
  // Mark floating-point truncating stores/extending loads as having custom
  // lowering
  if (VT.isFloatingPoint()) {
    MVT InnerVT = VT.changeVectorElementType(MVT::f16);
    while (InnerVT != VT) {
      setTruncStoreAction(VT, InnerVT, Custom);
      setLoadExtAction(ISD::EXTLOAD, VT, InnerVT, Default);
      InnerVT = InnerVT.changeVectorElementType(
          MVT::getFloatingPointVT(2 * InnerVT.getScalarSizeInBits()));
    }
  }
```

So, I firstly changed `if (VT.isFloatingPoint())` to `if (VT.isFloatingPoint() && !VT.isVectorOf(MVT::bf16))` in 3a6f32e806aef5f7a9770b4cd9cfc0700e04e684. And then I switched to `VT.getScalarType() == MVT::f32 || VT.getScalarType() == MVT::f64` as suggested when addressing the review comments. From my perspective, using `VT.getScalarType() == f32 || f64` is functionally equivalent and more specific than `isFloatingPoint` here, so I included this change in this PR.

Should I land this change in a separate PR in the future? I'm fine with that but I think it may be unnecessary.

https://github.com/llvm/llvm-project/pull/208744


More information about the llvm-commits mailing list