[SVE] Disable some BUILD_VECTOR related code generator features.

[paulwalker-arm]

Summary

Fixed length vector code generation for SVE does not yet custom
lower BUILD_VECTOR and instead relies on expansion. At the same
time custom lowering for VECTOR_SHUFFLE is also not available so
this patch disables using VECTOR_SHUFFLE to expand BUILD_VECTOR.

Related to this it also prevents the merging of stores after
legalisation because this only works when BUILD_VECTOR is either
legal or can be elminated. When this is not the case the code
generator enters an infinite legalisation loop.

[efriedma]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

3569

Would it be enough to just fix isShuffleMaskLegal, instead of overriding this?

llvm/lib/Target/AArch64/AArch64ISelLowering.h

745

This affects code that isn't using wide vectors, right?

If this is supposed to be a temporary hack, I guess it's fine, but please explicitly state that in the comment.

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

3569

We expand VECTOR_SHUFFLE (using BUILD_VECTOR) so at this stage I think it's better to just prevent extra VECTOR_SHUFFLE instances/code paths as early as possible.

llvm/lib/Target/AArch64/AArch64ISelLowering.h

745

Sadly it affects all vectors but the interface doesn't have the necessary information. I can see other targets have hit the same issue but I've restricted our version as best I can (only taking affect when wide vectors are enabled). I've added a comment saying we can revert the change once we fully support BUILD_VECTOR when using the wide vectors.

[paulwalker-arm]

Added comment saying when we can revert the mergeStoresAfterLegalization change.

[efriedma]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

3569

I'm not sure what "early" means in this context; the LegalizeDAG code that checks shouldExpandBuildVectorWithShuffles also checks isShuffleMaskLegal.

[paulwalker-arm]

Removed shouldExpandBuildVectorWithShuffles override and updated isShuffleMaskLegal instead.

[efriedma]

LGTM

[paulwalker-arm]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

3569

I was worried about the else clause but see that's unlikely to affect vectors >128bit. Updating isShuffleMaskLegal looks to work and check-llvm didn't highlight any unwanted side effects so this works for me.

llvm/lib/Target/AArch64/AArch64ISelLowering.h

public:
   bool functionArgumentNeedsConsecutiveRegisters(Type *Ty,
                                                  CallingConv::ID CallConv,
                                                  bool isVarArg) const override;
   /// Used for exception handling on Win64.
   bool needsFixedCatchObjects() const override;
 
   bool fallBackToDAGISel(const Instruction &Inst) const override;
 
+  /// SVE code generation for fixed length vectors does not custom lower
+  /// BUILD_VECTOR. This makes BUILD_VECTOR legalisation a source of stores to
+  /// merge. However, merging them creates a BUILD_VECTOR that is just as
+  /// illegal as the original, thus leading to an infinite legalisation loop.
+  /// NOTE: Once BUILD_VECTOR is legal or can be custom lowered for all legal
+  /// vector types this override can be removed.
+  bool mergeStoresAfterLegalization(EVT VT) const override {
+    return !useSVEForFixedLengthVectors();
+  }
+
 private:
   /// Keep a pointer to the AArch64Subtarget around so that we can
   /// make the right decision when generating code for different targets.
   const AArch64Subtarget *Subtarget;
 
   bool isExtFreeImpl(const Instruction *Ext) const override;
 
   void addTypeForNEON(MVT VT, MVT PromotedBitwiseVT);

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

case ISD::ADD:
     if (useSVEForFixedLengthVectorVT(Op.getValueType()))
       return LowerToPredicatedOp(Op, DAG, AArch64ISD::ADD_PRED);
     llvm_unreachable("Unexpected request to lower ISD::ADD");
   }
 }
 
 bool AArch64TargetLowering::useSVEForFixedLengthVectors() const {
   // Prefer NEON unless larger SVE registers are available.
   return Subtarget->hasSVE() && Subtarget->getMinSVEVectorSizeInBits() >= 256;
 }
 
 bool AArch64TargetLowering::useSVEForFixedLengthVectorVT(EVT VT) const {
   if (!useSVEForFixedLengthVectors())
     return false;
 
   if (!VT.isFixedLengthVector())
     return false;
SDValue AArch64TargetLowering::LowerINSERT_SUBVECTOR(SDValue Op,
   // This will be matched by custom code during ISelDAGToDAG.
   if (Idx == 0 && isPackedVectorType(InVT, DAG) && Op.getOperand(0).isUndef())
     return Op;
 
   return SDValue();
 }
 
 bool AArch64TargetLowering::isShuffleMaskLegal(ArrayRef<int> M, EVT VT) const {
+  // Currently no fixed length shuffles that require SVE are legal.
+  if (useSVEForFixedLengthVectorVT(VT))
+    return false;
+
   if (VT.getVectorNumElements() == 4 &&
       (VT.is128BitVector() || VT.is64BitVector())) {
     unsigned PFIndexes[4];
     for (unsigned i = 0; i != 4; ++i) {
       if (M[i] < 0)
         PFIndexes[i] = 8;
       else
         PFIndexes[i] = M[i];

llvm/test/CodeGen/AArch64/sve-fixed-length-shuffles.ll

 ; RUN: llc -aarch64-sve-vector-bits-min=256 < %s | FileCheck %s
 ; RUN: llc -aarch64-sve-vector-bits-min=512 < %s | FileCheck %s -check-prefixes=CHECK,VBITS_GE_512
 
 target triple = "aarch64-unknown-linux-gnu"
 
+; Currently there is no custom lowering for vector shuffles operating on types
+; bigger than NEON. However, having no support opens us up to a code generator
+; hang when expanding BUILD_VECTOR. Here we just validate the promblematic case
+; successfully exits code generation.
+define void @hang_when_merging_stores_after_legalisation(<8 x i32>* %a, <2 x i32> %b) #0 {
+; CHECK-LABEL: hang_when_merging_stores_after_legalisation:
+  %splat = shufflevector <2 x i32> %b, <2 x i32> undef, <8 x i32> zeroinitializer
+  %interleaved.vec = shufflevector <8 x i32> %splat, <8 x i32> undef, <8 x i32> <i32 0, i32 4, i32 1, i32 5, i32 2, i32 6, i32 3, i32 7>
+  store <8 x i32> %interleaved.vec, <8 x i32>* %a, align 4
+  ret void
+}
+
 ; NOTE: Currently all CONCAT_VECTORS get expanded so there's little point in
 ; validating all combinations of vector type.
 
 define void @concat_vectors_v4i64(<2 x i64> %a, <2 x i64> %b, <4 x i64> *%c.addr) #0 {
 ; CHECK-LABEL: concat_vectors_v4i64:
 ; CHECK: stp q0, q1, [sp]
 ; CHECK: ptrue [[OUT_PG:p[0-9]+]].d, vl4
 ; CHECK: mov x[[LO_ADDR:[0-9]+]], sp