[SVE] Lower fixed length VECREDUCE_SEQ_FADD operation

[cameron.mcinally]

Summary

@paulwalker-arm, I'm guessing this hits the reduction legalisation problems that @kmclaughlin is working on? This patch currently falls apart during splitting of the reduction nodes. If not, I'll continue to build it out.

Some other notes:

1. It looks like NEON FADDA support is missing upstream too.

2. We'll likely need to change how the OperationAction types are determined for the reduction-with-accumulator ISD nodes (see LegalizeDAG.cpp change). The types are currently based off the start_value type, not the vector op type. @sdesmalen

[paulwalker-arm]

Are you planning to add support for the normal VECREDUCE_FADD? I ask because that's likely to see more initial use upstream than the SEQ variant.

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

1154–1158

You'll need to move these below the main VECREDUCE_ options because VECREDUCE_FADD & VECREDUCE_FMUL only take a single operand.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

1131–1134

I doubt it's worth custom lowering v1f64 being the expansion will be a single neon ADD. Or is this because there is current no expand code of the SEQ reductions?

What about operations on vectors of f16.

16147

Given there's likely to be only one use of this I think calling this LowerVECREDUCE_SEQ_FADD is more inline with the naming of the other lowering functions.

16158–16161

Looks like we're missing some obvious patterns. I've created D89235 to add them.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h

223–224

This'll need to be more verbose because having SVE is only safe for scalable vectors. You'll need to look at the useSVEForFixedLengthVectorVT question if II is operating on fixed length types.

llvm/test/CodeGen/AArch64/sve-fixed-length-fp-reduce.ll

57

Looks like a stray X here.

[cameron.mcinally]

You're right on all the comments. I stopped midway when I hit the legalisation issue, so that's why this patch is rough. That would need to be built out first before this work could continue. And I think NEON support should be built out before that. It sounds like I'm not stepping on toes, so I'll go in that order. Thanks.

[cameron.mcinally]

Oh, and I can do VECREDUCE_FADD first. Just hit VECREDUCE_SEQ_FADD before I realized I need to add 'fast' to the intrinsic calls.

[cameron.mcinally]

This is ready for review now...

1. Notice that I made useSVEForFixedLengthVectors*(...) public members so that the ExpandReductions pass can access them. I don't keep up with C++ best practices, so I'm not sure if there's a better way to grant access to these functions. Any suggestions?

2. Also, expansion is kind of ugly for this operation. The ExpandReduction pass is an IR pass that runs before legalization, so the decision to expand is done early. It would be possible to not rely on ExpandReductions, and rather extend normal legalization to do an ordered lowering, but that would be duplicating what is already done. Does anyone feel strongly that expansion should be moved to Legalize or otherwise?

[cameron.mcinally]

Some other notes:

It looks like NEON FADDA support is missing upstream too.

I made a mistake here. I don't see an FADDA instruction in the NEON ISA, only SVE. Not sure why I thought otherwise...

[cameron.mcinally]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

16147

I moved LowerVECREDUCE_SEQ_FADD(...) out from under LowerVECREDUCE(...). Not sure if that helps or hurts @kmclaughlin, but I can change it if there's a problem.

[cameron.mcinally]

Fix 80 column issue. No other changes intended...

[cameron.mcinally]

Try again with 80 column fix...

[paulwalker-arm]

Sorry @cameron.mcinally I've not had much time for code reviews this week although will take proper look tomorrow. I have a question though. You've added extra legalisation support but I don't see any explicit tests (or at least ones with matching check lines) for it. Is this something you need for this patch? (I'm guessing sve-fixed-length-fp-reduce.ll's stock NEON run line triggers the cases?) If so then there really should be a neon specific test file that verifies the widening and scalarisation changes as the NEON run line for the "fixed-length" tests is more about ensuring no SVE instructions slip through.

[cameron.mcinally]

In D89162#2347701, @paulwalker-arm wrote:

Sorry @cameron.mcinally I've not had much time for code reviews this week although will take proper look tomorrow. I have a question though. You've added extra legalisation support but I don't see any explicit tests (or at least ones with matching check lines) for it. Is this something you need for this patch? (I'm guessing sve-fixed-length-fp-reduce.ll's stock NEON run line triggers the cases?) If so then there really should be a neon specific test file that verifies the widening and scalarisation changes as the NEON run line for the "fixed-length" tests is more about ensuring no SVE instructions slip through.

Thanks, Paul. No rush at all. Digressing a bit, we should probably sync-up on what else needs to be done for fixed length lowering. I believe this is the last operation on my list.

The legalisation changes are specific to VECREDUCE_SEQ_FADD on SVE. Prior to this patch, VECREDUCE_SEQ_FADD is expanded in the ExpandReductions pass before legalisation runs, so VECREDUCE_SEQ_FADD would never reach legalisation on NEON. (That's assuming NEON has no FADDA support. I might be wrong about that.)

The new tests would be broken without the legalisation changes, so I'm assuming that those are enough coverage. Maybe I'm missing something though...

[paulwalker-arm]

In D89162#2347814, @cameron.mcinally wrote:

The new tests would be broken without the legalisation changes, so I'm assuming that those are enough coverage. Maybe I'm missing something though...

Are you sure? I took your patch for a test drive and removed all but the TLI.getOperationAction related change from Legalize*.{cpp, h} and the tests passed.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

16170

I think this can be ContainerVT?

16174

You use this in a couple of places, so might be worth extracting into Zero = ?

llvm/test/CodeGen/AArch64/sve-fixed-length-fp-reduce.ll

65

What does this mean?

I was going to say you're missing VBITS_EQ_256 lines here but then noticed the output. What's you're feeling on this? I see two routes:

Stick with the expanded output for this patch, in which case just having VBITS_EQ_256-COUNT-33: fadd is good enough.

For better code generation we're just missing the ability to split VECREDUCE_SEQ_FADD operations, which I don't think should be that hard.

VECREDUCE_SEQ_FADD A, X_512 => VECREDUCE_SEQ_FADD (VECREDUCE_SEQ_FADD X, X_LO), X_HI ?

I'm happy with either approach.

[cameron.mcinally]

In D89162#2350199, @paulwalker-arm wrote:

In D89162#2347814, @cameron.mcinally wrote:

The new tests would be broken without the legalisation changes, so I'm assuming that those are enough coverage. Maybe I'm missing something though...

Are you sure? I took your patch for a test drive and removed all but the TLI.getOperationAction related change from Legalize*.{cpp, h} and the tests passed.

That doesn't sound right. E.g. we need the scalarization code for <1 x f64>. I'll check again...

llvm/test/CodeGen/AArch64/sve-fixed-length-fp-reduce.ll

65

For better code generation we're just missing the ability to split VECREDUCE_SEQ_FADD operations, which I don't think should be that hard.

Oh, you're right. I initially thought we couldn't preserve the ordering constraint if we split, but we could use the accumulator to chain them. Makes sense. I'll look into that...

[nikic]

This should be split into two parts:

1. A patch that implements all the necessary legalizations for VECREDUCE_SEQ_* and enables use of SDAG legalization on AArch64 (and possibly ARM) unconditionally. This may need some additional test coverage to trigger all possible legalizations. Necessary legalizations also include float legalization, not just vector legalization.
2. A patch that adds improved lowerings using SVE instructions based on that.

[paulwalker-arm]

@nikic Why does wanting to implement VECREDUCE_SEQ_FADD for SVE necessitate having to implement full support for NEON (AArch64 and Arm)?

[nikic]

@paulwalker-arm There is no need to implement any target-specific support. The legalization outcome will be a simple chain of extracts and fadd/fmuls. It does not need to generate good code, just not assert for any VTs.

[paulwalker-arm]

In D89162#2350313, @nikic wrote:

@paulwalker-arm There is no need to implement any target-specific support. The legalization outcome will be a simple chain of extracts and fadd/fmuls. It does not need to generate good code, just not assert for any VTs.

Sure, I think there's been a misunderstanding. The only target-specific piece we're talking about is the existing shouldExpandReduction hook that controls if the code generator will see the VECREDUCE_SEQ_FADD to legalise. So I'm wondering if an acceptable ordering is to allow the lowering of legal VECREDUCE_SEQ_FADD operations (which is only what shouldExpandReduction will let through) and then we can tackle the type legalisation problem second so that shouldExpandReduction can let everything through when SVE is enable (of which they'll already be a test ready and waiting) and leave NEON as it is today.

[cameron.mcinally]

Updating patch, but not ready for a serious review yet as I haven't started the splitting work. I'm still not convinced we can handle splitting appropriately with the current setup, but will comment on that seperately.

I caused a big misunderstanding here, so let me try to unwind it. Paul is correct. The legalizations are NOT necessary for SVE support. I made a mistake when preparing this patch. I.e. I built the legalization changes out before I finalized the shouldExpandReduction(...) changes, so ended up in a weird state with NEON.

The ExpandReductions pass is somewhat unusual and that threw me off. It's an IR pass that runs early in llc, before legalisation. I've never seen one of those, so it tripped me up. I apologize for the confusion. That said, I wonder if the reductions should really be expanded during legalization. That would have been a more natural choice to me.

[paulwalker-arm]

@cameron.mcinally: I'm sure you know this but just in case it saves some time I can confirm you will not hit the splitting code until after you relax shouldExpandReduction. So I think your current patch is complete so it really comes down to whether adding the support this way round (i.e. only allow legal types for SVE, then allow all types and implement the legalisation) is acceptable to @nikic .

[nikic]

Thanks for the clarification, I indeed misunderstood this. If you have a way to accurately pick out the legal types, then this is of course fine. My only concern was to not introduce incomplete legalization support for these opcodes.

The ExpandReductions pass is somewhat unusual and that threw me off. It's an IR pass that runs early in llc, before legalisation. I've never seen one of those, so it tripped me up. I apologize for the confusion. That said, I wonder if the reductions should really be expanded during legalization. That would have been a more natural choice to me.

The eventual goal here is to expand reductions during legalization only. The IR expansion exists because the DAG legalization support has been patchy historically, with VECREDUCE_SEQ_* being the last remaining hole.

[cameron.mcinally]

In D89162#2350619, @nikic wrote:

The eventual goal here is to expand reductions during legalization only. The IR expansion exists because the DAG legalization support has been patchy historically, with VECREDUCE_SEQ_* being the last remaining hole.

Ah, ok. I'm happy to hear we're all in agreement. I might be able to volunteer some time for that work, but it's not really my prime directive.

Again, apologies for the mix up. There were a lot of moving parts for this patch and I dropped the ball. *^_^*

[cameron.mcinally]

@paulwalker-arm, back to the splitting discussion...

Looks like we need to wait for the full VECREDUCE_SEQ_FADD legalization changes before the splitting work can be done. We could make changes to shouldExpandReduction(), to recognize large vectors that could be split, but I suspect that code will end up too hacky.

Unless I'm missing something. What are your thoughts?

[paulwalker-arm]

With this patch[1] landed I believe operation legalisation is now a solved problem for SVE (well for fixed length vectors). I think it's worth tackling the type legalisation side of things rather than overcomplicating shouldExpandReduction. Of course that's easy for me to say given it's your time :) but you've already done part of the work based on the older patch.

Your WidenVecOp_VECREDUCE changes looked good and we've already discussed how splitting can be done in a generic way. With these two pieces I think shouldExpandReduction just needs to test for SVE.

When that lands we hit another decision point to decide whether it's worth avoiding SVE for very small vectors and thus whether to implement the scalarisation.

[1] I just wanted to highlight my previous VBITS_EQ_256-COUNT-33: fadd comment as this gives us a bit more test coverage and is something that will obviously fail (in a good way) when the splitting work is available.

[cameron.mcinally]

[1] I just wanted to highlight my previous VBITS_EQ_256-COUNT-33: fadd comment as this gives us a bit more test coverage and is something that will obviously fail (in a good way) when the splitting work is available.

Got it. Also added a FIXME to remind us to move the useSVEForFixedLengthVectors*() functions back to private scope when the reduction legalization is complete.

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

Action = TLI.getOperationAction(Node->getOpcode(),
                     cast<MaskedScatterSDNode>(Node)->getValue().getValueType());
     break;
   case ISD::MSTORE:
     Action = TLI.getOperationAction(Node->getOpcode(),
                     cast<MaskedStoreSDNode>(Node)->getValue().getValueType());
     break;
   case ISD::VECREDUCE_FADD:
   case ISD::VECREDUCE_FMUL:
+  case ISD::VECREDUCE_SEQ_FADD:
+  case ISD::VECREDUCE_SEQ_FMUL:
+    Action = TLI.getOperationAction(
+        Node->getOpcode(), Node->getOperand(1).getValueType());
+    break;
   case ISD::VECREDUCE_ADD:
   case ISD::VECREDUCE_MUL:
   case ISD::VECREDUCE_AND:
   case ISD::VECREDUCE_OR:
   case ISD::VECREDUCE_XOR:
   case ISD::VECREDUCE_SMAX:
   case ISD::VECREDUCE_SMIN:
   case ISD::VECREDUCE_UMAX:

llvm/lib/Target/AArch64/AArch64ISelLowering.h

private:
   SDValue LowerSVEStructLoad(unsigned Intrinsic, ArrayRef<SDValue> LoadOps,
                              EVT VT, SelectionDAG &DAG, const SDLoc &DL) const;
 
   SDValue LowerFixedLengthVectorIntDivideToSVE(SDValue Op,
                                                SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorIntExtendToSVE(SDValue Op,
                                                SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorLoadToSVE(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerFixedLengthReductionAccToSVE(unsigned Opcode, SDValue ScalarOp,
+                                            SelectionDAG &DAG) const;
   SDValue LowerFixedLengthReductionToSVE(unsigned Opcode, SDValue ScalarOp,
                                          SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorSelectToSVE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorSetccToSVE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorStoreToSVE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFixedLengthVectorTruncateToSVE(SDValue Op,
                                               SelectionDAG &DAG) const;
 

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

if (useSVEForFixedLengthVectors()) {
       setOperationAction(ISD::UMAX, MVT::v1i64, Custom);
       setOperationAction(ISD::UMAX, MVT::v2i64, Custom);
       setOperationAction(ISD::UMIN, MVT::v1i64, Custom);
       setOperationAction(ISD::UMIN, MVT::v2i64, Custom);
       setOperationAction(ISD::VECREDUCE_SMAX, MVT::v2i64, Custom);
       setOperationAction(ISD::VECREDUCE_SMIN, MVT::v2i64, Custom);
       setOperationAction(ISD::VECREDUCE_UMAX, MVT::v2i64, Custom);
       setOperationAction(ISD::VECREDUCE_UMIN, MVT::v2i64, Custom);
+
+      // Int operations with no NEON 64b/128b support.
       for (auto VT : {MVT::v8i8, MVT::v16i8, MVT::v4i16, MVT::v8i16,
                       MVT::v2i32, MVT::v4i32, MVT::v2i64}) {
         setOperationAction(ISD::VECREDUCE_AND, VT, Custom);
         setOperationAction(ISD::VECREDUCE_OR, VT, Custom);
         setOperationAction(ISD::VECREDUCE_XOR, VT, Custom);
       }
+
+      // FP operations with no NEON 64b/128b support.
+      for (auto VT : {MVT::v2f32, MVT::v4f32, MVT::v1f64, MVT::v2f64})
+        // FIXME: There is a NEON FADDA instruction, but it's not
+        //        currently supported.
+        setOperationAction(ISD::VECREDUCE_SEQ_FADD, VT, Custom);
     }
   }
 
   PredictableSelectIsExpensive = Subtarget->predictableSelectIsExpensive();
 }
 
 void AArch64TargetLowering::addTypeForNEON(MVT VT, MVT PromotedBitwiseVT) {
   assert(VT.isVector() && "VT should be a vector type");
void AArch64TargetLowering::addTypeForFixedLengthSVE(MVT VT) {
   setOperationAction(ISD::UDIV, VT, Custom);
   setOperationAction(ISD::UMAX, VT, Custom);
   setOperationAction(ISD::UMIN, VT, Custom);
   setOperationAction(ISD::VECREDUCE_ADD, VT, Custom);
   setOperationAction(ISD::VECREDUCE_AND, VT, Custom);
   setOperationAction(ISD::VECREDUCE_FMAX, VT, Custom);
   setOperationAction(ISD::VECREDUCE_FMIN, VT, Custom);
   setOperationAction(ISD::VECREDUCE_OR, VT, Custom);
+  setOperationAction(ISD::VECREDUCE_SEQ_FADD, VT, Custom);
   setOperationAction(ISD::VECREDUCE_SMAX, VT, Custom);
   setOperationAction(ISD::VECREDUCE_SMIN, VT, Custom);
   setOperationAction(ISD::VECREDUCE_UMAX, VT, Custom);
   setOperationAction(ISD::VECREDUCE_UMIN, VT, Custom);
   setOperationAction(ISD::VECREDUCE_XOR, VT, Custom);
   setOperationAction(ISD::VSELECT, VT, Custom);
   setOperationAction(ISD::XOR, VT, Custom);
   setOperationAction(ISD::ZERO_EXTEND, VT, Custom);
SDValue AArch64TargetLowering::LowerOperation(SDValue Op,
   case ISD::VECREDUCE_ADD:
   case ISD::VECREDUCE_AND:
   case ISD::VECREDUCE_OR:
   case ISD::VECREDUCE_XOR:
   case ISD::VECREDUCE_SMAX:
   case ISD::VECREDUCE_SMIN:
   case ISD::VECREDUCE_UMAX:
   case ISD::VECREDUCE_UMIN:
+  case ISD::VECREDUCE_SEQ_FADD:
   case ISD::VECREDUCE_FMAX:
   case ISD::VECREDUCE_FMIN:
     return LowerVECREDUCE(Op, DAG);
   case ISD::ATOMIC_LOAD_SUB:
     return LowerATOMIC_LOAD_SUB(Op, DAG);
   case ISD::ATOMIC_LOAD_AND:
     return LowerATOMIC_LOAD_AND(Op, DAG);
   case ISD::DYNAMIC_STACKALLOC:
static SDValue getReductionSDNode(unsigned Op, SDLoc DL, SDValue ScalarOp,
   SDValue VecOp = ScalarOp.getOperand(0);
   auto Rdx = DAG.getNode(Op, DL, VecOp.getSimpleValueType(), VecOp);
   return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, ScalarOp.getValueType(), Rdx,
                      DAG.getConstant(0, DL, MVT::i64));
 }
 
 SDValue AArch64TargetLowering::LowerVECREDUCE(SDValue Op,
                                               SelectionDAG &DAG) const {
-  SDValue Src = Op.getOperand(0);
+  // The first argument of VECREDUCE_SEQ_FADD is the initial value.
+  SDValue Src = (Op.getOpcode() == ISD::VECREDUCE_SEQ_FADD) ? Op.getOperand(1)
+                                                            : Op.getOperand(0);
 
   // Try to lower fixed length reductions to SVE.
   EVT SrcVT = Src.getValueType();
-  bool OverrideNEON = Op.getOpcode() == ISD::VECREDUCE_AND ||
+  bool OverrideNEON = Op.getOpcode() == ISD::VECREDUCE_SEQ_FADD ||
+                      Op.getOpcode() == ISD::VECREDUCE_AND ||
                       Op.getOpcode() == ISD::VECREDUCE_OR ||
                       Op.getOpcode() == ISD::VECREDUCE_XOR ||
                       (Op.getOpcode() != ISD::VECREDUCE_ADD &&
                        SrcVT.getVectorElementType() == MVT::i64);
   if (useSVEForFixedLengthVectorVT(SrcVT, OverrideNEON)) {
     switch (Op.getOpcode()) {
     case ISD::VECREDUCE_ADD:
       return LowerFixedLengthReductionToSVE(AArch64ISD::UADDV_PRED, Op, DAG);
     case ISD::VECREDUCE_AND:
       return LowerFixedLengthReductionToSVE(AArch64ISD::ANDV_PRED, Op, DAG);
     case ISD::VECREDUCE_OR:
       return LowerFixedLengthReductionToSVE(AArch64ISD::ORV_PRED, Op, DAG);
     case ISD::VECREDUCE_SMAX:
       return LowerFixedLengthReductionToSVE(AArch64ISD::SMAXV_PRED, Op, DAG);
     case ISD::VECREDUCE_SMIN:
       return LowerFixedLengthReductionToSVE(AArch64ISD::SMINV_PRED, Op, DAG);
     case ISD::VECREDUCE_UMAX:
       return LowerFixedLengthReductionToSVE(AArch64ISD::UMAXV_PRED, Op, DAG);
     case ISD::VECREDUCE_UMIN:
       return LowerFixedLengthReductionToSVE(AArch64ISD::UMINV_PRED, Op, DAG);
     case ISD::VECREDUCE_XOR:
       return LowerFixedLengthReductionToSVE(AArch64ISD::EORV_PRED, Op, DAG);
+    case ISD::VECREDUCE_SEQ_FADD:
+      return LowerFixedLengthReductionAccToSVE(AArch64ISD::FADDA_PRED, Op, DAG);
     case ISD::VECREDUCE_FMAX:
       return LowerFixedLengthReductionToSVE(AArch64ISD::FMAXNMV_PRED, Op, DAG);
     case ISD::VECREDUCE_FMIN:
       return LowerFixedLengthReductionToSVE(AArch64ISD::FMINNMV_PRED, Op, DAG);
     default:
       llvm_unreachable("Unhandled fixed length reduction");
     }
   }
assert(useSVEForFixedLengthVectorVT(V.getValueType()) &&
            "Only fixed length vectors are supported!");
     Ops.push_back(convertToScalableVector(DAG, ContainerVT, V));
   }
 
   auto ScalableRes = DAG.getNode(Op.getOpcode(), SDLoc(Op), ContainerVT, Ops);
   return convertFromScalableVector(DAG, VT, ScalableRes);
 }
 
+SDValue AArch64TargetLowering::LowerFixedLengthReductionAccToSVE(
+    unsigned Opcode, SDValue ScalarOp, SelectionDAG &DAG) const {
+  SDLoc DL(ScalarOp);
+  SDValue AccOp = ScalarOp.getOperand(0);
+  SDValue VecOp = ScalarOp.getOperand(1);
+
+  // Convert operands to Scalable.
+  EVT SrcVT = VecOp.getValueType();
+  EVT ResVT = SrcVT.getVectorElementType();
+  SDValue Pg = getPredicateForVector(DAG, DL, SrcVT);
+  EVT ContainerVT = getContainerForFixedLengthVector(DAG, SrcVT);
+  // FIXME: This produces a masked MOV. Do we care about the false lanes?
+  AccOp = DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, ContainerVT,
+                      DAG.getUNDEF(ContainerVT), AccOp,
+                      DAG.getConstant(0, DL, MVT::i64));
+  VecOp = convertToScalableVector(DAG, ContainerVT, VecOp);
+
+  // Perform reduction.
+  SDValue Rdx = DAG.getNode(Opcode, DL, getPackedSVEVectorVT(ResVT), Pg,
+                            AccOp, VecOp);
+
+  return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, ResVT, Rdx,
+                     DAG.getConstant(0, DL, MVT::i64));
+}
+
 SDValue AArch64TargetLowering::LowerFixedLengthReductionToSVE(unsigned Opcode,
     SDValue ScalarOp, SelectionDAG &DAG) const {
   SDLoc DL(ScalarOp);
   SDValue VecOp = ScalarOp.getOperand(0);
   EVT SrcVT = VecOp.getValueType();
 
   SDValue Pg = getPredicateForVector(DAG, DL, SrcVT);
   EVT ContainerVT = getContainerForFixedLengthVector(DAG, SrcVT);
   VecOp = convertToScalableVector(DAG, ContainerVT, VecOp);
 
   // UADDV always returns an i64 result.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h

public:
 
   bool
   shouldConsiderAddressTypePromotion(const Instruction &I,
                                      bool &AllowPromotionWithoutCommonHeader);
 
   bool shouldExpandReduction(const IntrinsicInst *II) const {
     switch (II->getIntrinsicID()) {
     case Intrinsic::vector_reduce_fadd:
+      if (ST->hasSVE())
+        return false;
+    LLVM_FALLTHROUGH;
     case Intrinsic::vector_reduce_fmul:
       // We don't have legalization support for ordered FP reductions.
       return !II->getFastMathFlags().allowReassoc();
 
     default:
       // Don't expand anything else, let legalization deal with it.
       return false;
     }

llvm/test/CodeGen/AArch64/sve-fixed-length-fp-reduce.ll

 ; RUN: llc -aarch64-sve-vector-bits-min=2048 -asm-verbose=0 < %s | FileCheck %s -D#VBYTES=256 -check-prefixes=CHECK,VBITS_GE_512,VBITS_GE_1024,VBITS_GE_2048
 
 target triple = "aarch64-unknown-linux-gnu"
 
 ; Don't use SVE when its registers are no bigger than NEON.
 ; NO_SVE-NOT: ptrue
 
 ;
+; FADDA
+;
+
+; NEON 64-bit FADDA not currently supported.
+define float @fadda_v2f32(float %start, <2 x float> %a) #0 {
+; CHECK-LABEL: fadda_v2f32:
+; CHECK: ptrue [[PG:p[0-9]+]].s, vl1
+; CHECK-NEXT: mov z[[ACC:[0-9]+]].s, [[PG]]/m, z0.s
+; CHECK-NEXT: ptrue [[PG1:p[0-9]+]].s, vl2
+; CHECK-NEXT: fadda s0, [[PG1]], s[[ACC]], z1.s
+; CHECK-NEXT: ret
+  %res = call float @llvm.vector.reduce.fadd.v2f32(float %start, <2 x float> %a)
+  ret float %res
+}
+
+; NEON 128-bit FADDA not currently supported.
+define float @fadda_v4f32(float %start, <4 x float> %a) #0 {
+; CHECK-LABEL: fadda_v4f32:
+; CHECK: ptrue [[PG1:p[0-9]+]].s, vl1
+; CHECK-NEXT: mov z[[ACC:[0-9]+]].s, [[PG1]]/m, z0.s
+; CHECK-NEXT: ptrue [[PG:p[0-9]+]].s, vl4
+; CHECK-NEXT: fadda s0, [[PG]], s[[ACC]], z1.s
+; CHECK-NEXT: ret
+  %res = call float @llvm.vector.reduce.fadd.v4f32(float %start, <4 x float> %a)
+  ret float %res
+}
+
+define float @fadda_v8f32(float %start, <8 x float>* %a) #0 {
+; CHECK-LABEL: fadda_v8f32:
+; VBITS_GE_256: ptrue [[PG:p[0-9]+]].s, vl8
+; VBITS_GE_256-NEXT: ld1w { [[OP:z[0-9]+]].s }, [[PG]]/z, [x0]
+; VBITS_GE_256-NEXT: ptrue [[PG1:p[0-9]+]].s, vl1
+; VBITS_GE_256-NEXT: mov z[[ACC:[0-9]+]].s, [[PG1]]/m, z0.s
+; VBITS_GE_256-NEXT: Xfadda s0, [[PG]], s[[ACC]], [[OP]].s
+; VBITS_GE_256-NEXT: ret
+  %op = load <8 x float>, <8 x float>* %a
+  %res = call float @llvm.vector.reduce.fadd.v8f32(float %start, <8 x float> %op)
+  ret float %res
+}
+
+define float @fadda_v16f32(float %start, <16 x float>* %a) #0 {
+; CHECK-LABEL: fadda_v16f32:
+; VBITS_GE_512: ptrue [[PG:p[0-9]+]].s, vl16
+; VBITS_GE_512-NEXT: ld1w { [[OP:z[0-9]+]].s }, [[PG]]/z, [x0]
+; VBITS_GE_512-NEXT: ptrue [[PG1:p[0-9]+]].s, vl1
+; VBITS_GE_512-NEXT: mov z[[ACC:[0-9]+]].s, [[PG1]]/m, z0.s
+; VBITS_GE_512-NEXT: fadda s0, [[PG]], s[[ACC]], [[OP]].s
+; VBITS_GE_512-NEXT: ret
+
+; Ensure sensible type legalisation.
+; FIXME: Legalisation is broken.
+  %op = load <16 x float>, <16 x float>* %a
+  %res = call float @llvm.vector.reduce.fadd.v16f32(float %start, <16 x float> %op)
+  ret float %res
+}
+
+define float @fadda_v32f32(float %start, <32 x float>* %a) #0 {
+; CHECK-LABEL: fadda_v32f32:
+; VBITS_GE_1024: ptrue [[PG:p[0-9]+]].s, vl32
+; VBITS_GE_1024-NEXT: ld1w { [[OP:z[0-9]+]].s }, [[PG]]/z, [x0]
+; VBITS_GE_1024-NEXT: ptrue [[PG1:p[0-9]+]].s, vl1
+; VBITS_GE_1024-NEXT: mov z[[ACC:[0-9]+]].s, [[PG1]]/m, z0.s
+; VBITS_GE_1024-NEXT: fadda s0, [[PG]], s[[ACC]], [[OP]].s
+; VBITS_GE_1024-NEXT: ret
+  %op = load <32 x float>, <32 x float>* %a
+  %res = call float @llvm.vector.reduce.fadd.v32f32(float %start, <32 x float> %op)
+  ret float %res
+}
+
+define float @fadda_v64f32(float %start, <64 x float>* %a) #0 {
+; CHECK-LABEL: fadda_v64f32:
+; VBITS_GE_2048: ptrue [[PG:p[0-9]+]].s, vl64
+; VBITS_GE_2048-NEXT: ld1w { [[OP:z[0-9]+]].s }, [[PG]]/z, [x0]
+; VBITS_GE_2048-NEXT: ptrue [[PG1:p[0-9]+]].s, vl1
+; VBITS_GE_2048-NEXT: mov z[[ACC:[0-9]+]].s, [[PG1]]/m, z0.s
+; VBITS_GE_2048-NEXT: fadda s0, [[PG]], s[[ACC]], [[OP]].s
+; VBITS_GE_2048-NEXT: ret
+  %op = load <64 x float>, <64 x float>* %a
+  %res = call float @llvm.vector.reduce.fadd.v64f32(float %start, <64 x float> %op)
+  ret float %res
+}
+
+; NEON 64-bit FADDA not currently supported.
+define double @fadda_v1f64(double %start, <1 x double> %a) #0 {
+; CHECK-LABEL: fadda_v1f64:
+; CHECK: ptrue [[PG:p[0-9]+]].d, vl1
+; CHECK-NEXT: mov z[[ACC:[0-9]+]].d, [[PG]]/m, z0.d
+; CHECK-NEXT: fadda d0, [[PG]], d[[ACC]], z1.d
+; CHECK-NEXT: ret
+  %res = call double @llvm.vector.reduce.fadd.v1f64(double %start, <1 x double> %a)
+  ret double %res
+}
+
+; NEON 128-bit FADDA not currently supported.
+define double @fadda_v2f64(double %start, <2 x double> %a) #0 {
+; CHECK-LABEL: fadda_v2f64:
+; CHECK: ptrue [[PG1:p[0-9]+]].d, vl1
+; CHECK-NEXT: mov z[[ACC:[0-9]+]].d, [[PG1]]/m, z0.d
+; CHECK-NEXT: ptrue [[PG:p[0-9]+]].d, vl2
+; CHECK-NEXT: fadda d0, [[PG]], d[[ACC]], z1.d
+; CHECK-NEXT: ret
+  %res = call double @llvm.vector.reduce.fadd.v2f64(double %start, <2 x double> %a)
+  ret double %res
+}
+
+define double @fadda_v4f64(double %start, <4 x double>* %a) #0 {
+; CHECK-LABEL: fadda_v4f64:
+; VBITS_GE_256: ptrue [[PG:p[0-9]+]].d, vl4
+; VBITS_GE_256-NEXT: ld1d { [[OP:z[0-9]+]].d }, [[PG]]/z, [x0]
+; VBITS_GE_256-NEXT: ptrue [[PG1:p[0-9]+]].d, vl1
+; VBITS_GE_256-NEXT: mov z[[ACC:[0-9]+]].d, [[PG1]]/m, z0.d
+; VBITS_GE_256-NEXT: fadda d0, [[PG]], d[[ACC]], [[OP]].d
+; VBITS_GE_256-NEXT: ret
+  %op = load <4 x double>, <4 x double>* %a
+  %res = call double @llvm.vector.reduce.fadd.v4f64(double %start, <4 x double> %op)
+  ret double %res
+}
+
+define double @fadda_v8f64(double %start, <8 x double>* %a) #0 {
+; CHECK-LABEL: fadda_v8f64:
+; VBITS_GE_512: ptrue [[PG:p[0-9]+]].d, vl8
+; VBITS_GE_512-NEXT: ld1d { [[OP:z[0-9]+]].d }, [[PG]]/z, [x0]
+; VBITS_GE_512-NEXT: ptrue [[PG1:p[0-9]+]].d, vl1
+; VBITS_GE_512-NEXT: mov z[[ACC:[0-9]+]].d, [[PG1]]/m, z0.d
+; VBITS_GE_512-NEXT: fadda d0, [[PG]], d[[ACC]], [[OP]].d
+; VBITS_GE_512-NEXT: ret
+
+; Ensure sensible type legalisation.
+; FIXME: Legalisation is broken.
+  %op = load <8 x double>, <8 x double>* %a
+  %res = call double @llvm.vector.reduce.fadd.v8f64(double %start, <8 x double> %op)
+  ret double %res
+}
+
+define double @fadda_v16f64(double %start, <16 x double>* %a) #0 {
+; CHECK-LABEL: fadda_v16f64:
+; VBITS_GE_1024: ptrue [[PG:p[0-9]+]].d, vl16
+; VBITS_GE_1024-NEXT: ld1d { [[OP:z[0-9]+]].d }, [[PG]]/z, [x0]
+; VBITS_GE_1024-NEXT: ptrue [[PG1:p[0-9]+]].d, vl1
+; VBITS_GE_1024-NEXT: mov z[[ACC:[0-9]+]].d, [[PG1]]/m, z0.d
+; VBITS_GE_1024-NEXT: fadda d0, [[PG]], d[[ACC]], [[OP]].d
+; VBITS_GE_1024-NEXT: ret
+  %op = load <16 x double>, <16 x double>* %a
+  %res = call double @llvm.vector.reduce.fadd.v16f64(double %start, <16 x double> %op)
+  ret double %res
+}
+
+define double @fadda_v32f64(double %start, <32 x double>* %a) #0 {
+; CHECK-LABEL: fadda_v32f64:
+; VBITS_GE_2048: ptrue [[PG:p[0-9]+]].d, vl32
+; VBITS_GE_2048-NEXT: ld1d { [[OP:z[0-9]+]].d }, [[PG]]/z, [x0]
+; VBITS_GE_2048-NEXT: ptrue [[PG1:p[0-9]+]].d, vl1
+; VBITS_GE_2048-NEXT: mov z[[ACC:[0-9]+]].d, [[PG1]]/m, z0.d
+; VBITS_GE_2048-NEXT: fadda d0, [[PG]], d[[ACC]], [[OP]].d
+; VBITS_GE_2048-NEXT: ret
+  %op = load <32 x double>, <32 x double>* %a
+  %res = call double @llvm.vector.reduce.fadd.v32f64(double %start, <32 x double> %op)
+  ret double %res
+}
+
+;
 ; FMAXV
 ;
 
 ; No NEON 16-bit vector FMAXNMV support. Use SVE.
 define half @fmaxv_v4f16(<4 x half> %a) #0 {
 ; CHECK-LABEL: fmaxv_v4f16:
 ; CHECK: fmaxnmv h0, v0.4h
 ; CHECK-NEXT: ret

 ; VBITS_GE_2048-NEXT: ret
   %op = load <32 x double>, <32 x double>* %a
   %res = call double @llvm.vector.reduce.fmin.v32f64(<32 x double> %op)
   ret double %res
 }
 
 attributes #0 = { "target-features"="+sve" }
 
+declare float @llvm.vector.reduce.fadd.v2f32(float, <2 x float>)
+declare float @llvm.vector.reduce.fadd.v4f32(float, <4 x float>)
+declare float @llvm.vector.reduce.fadd.v8f32(float, <8 x float>)
+declare float @llvm.vector.reduce.fadd.v16f32(float, <16 x float>)
+declare float @llvm.vector.reduce.fadd.v32f32(float, <32 x float>)
+declare float @llvm.vector.reduce.fadd.v64f32(float, <64 x float>)
+
+declare double @llvm.vector.reduce.fadd.v1f64(double, <1 x double>)
+declare double @llvm.vector.reduce.fadd.v2f64(double, <2 x double>)
+declare double @llvm.vector.reduce.fadd.v4f64(double, <4 x double>)
+declare double @llvm.vector.reduce.fadd.v8f64(double, <8 x double>)
+declare double @llvm.vector.reduce.fadd.v16f64(double, <16 x double>)
+declare double @llvm.vector.reduce.fadd.v32f64(double, <32 x double>)
+
 declare half @llvm.vector.reduce.fmax.v4f16(<4 x half>)
 declare half @llvm.vector.reduce.fmax.v8f16(<8 x half>)
 declare half @llvm.vector.reduce.fmax.v16f16(<16 x half>)
 declare half @llvm.vector.reduce.fmax.v32f16(<32 x half>)
 declare half @llvm.vector.reduce.fmax.v64f16(<64 x half>)
 declare half @llvm.vector.reduce.fmax.v128f16(<128 x half>)
 
 declare float @llvm.vector.reduce.fmax.v2f32(<2 x float>)