[PATCH] D95245: [SVE] Add support for scalable vectorization of loops with int/fast FP reductions

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1103
                                            TTI::ReductionFlags Flags) const {
+  if (isa<ScalableVectorType>(Ty))
+    return true;
----------------
david-arm wrote:
> Just a thought - if we're excluding FMul from reductions is it worth having an assert here that the op is not fmul?
The same should hold for integer Mul.

nit: you can better add that to the switch statement below as:
  case Instruction::Mul:
  case Instruction::FMul:
    assert(!isa<ScalableVectorType>(Ty) && "Unexpected ...");
    LLVM_FALLTHROUGH;
  case Instruction::Fadd:
  ...



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:189
 
+  bool isLegalScalarTypeForSVE(Type *Ty) const {
+    if (Ty->isBFloatTy() && ST->hasBF16())
----------------
Can you merge this function with `isLegalScalarTypeForSVEMaskedMemOp` and name it `isLegalElementTypeForSVE`?

I think their implementation should be the same (including your check here for `hasBF16`)


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

https://reviews.llvm.org/D95245