[ARM][MVE] Tail predicate VML[A|S]LDAV

[samparker]

Summary

Make the non-exchanging versions of the multiply add/sub instructions validForTailPredication.

[yroux]

llvm/unittests/Target/ARM/MachineInstrTest.cpp

371

Maybe this comment can be updated, since some of these ops are allowed.

[dmgreen]

I think you said this morning that this is OK because we also mark them as horizontal, so do extra checks on them to make sure that the "off" lanes are all 0? Can you add a comment somewhere explaining that kind of thing?

[samparker]

Added/updated comments.

[yroux]

Ok thanks, LGTM

[dmgreen]

Yep. Thanks

llvm/lib/Target/ARM/ARMInstrMVE.td

   let Inst{15-13} = RdaDest{3-1};
   let Inst{12} = X;
   let Inst{8} = bit_8;
   let Inst{7-6} = 0b00;
   let Inst{5} = A;
   let Inst{3-1} = Qm{2-0};
   let Inst{0} = bit_0;
   let horizontalReduction = 1;
+  // Allow tail predication for non-exchanging versions. As this is also a
+  // horizontalReduction, ARMLowOverheadLoops will also have to check that
+  // the vector operands contain zeros in their false lanes for the instruction
+  // to be properly valid.
+  let validForTailPredication = !eq(X, 0);
 }
 
 multiclass MVE_VMLAMLSDAV_A<string iname, string x, MVEVectorVTInfo VTI,
                             bit sz, bit bit_28, bit X, bit bit_8, bit bit_0> {
   def ""#x#VTI.Suffix : MVE_VMLAMLSDAV<iname # x, VTI.Suffix,
                                    (ins MQPR:$Qn, MQPR:$Qm), "",
                                    sz, bit_28, 0b0, X, bit_8, bit_0>;
   def "a"#x#VTI.Suffix : MVE_VMLAMLSDAV<iname # "a" # x, VTI.Suffix,

   let Inst{15-13} = RdaLoDest{3-1};
   let Inst{12} = X;
   let Inst{8} = bit_8;
   let Inst{7-6} = 0b00;
   let Inst{5} = A;
   let Inst{3-1} = Qm{2-0};
   let Inst{0} = bit_0;
   let horizontalReduction = 1;
+  // Allow tail predication for non-exchanging versions. As this is also a
+  // horizontalReduction, ARMLowOverheadLoops will also have to check that
+  // the vector operands contain zeros in their false lanes for the instruction
+  // to be properly valid.
+  let validForTailPredication = !eq(X, 0);
 
   let hasSideEffects = 0;
 }
 
 multiclass MVE_VMLALDAVBase_A<string iname, string x, string suffix,
                                bit sz, bit bit_28, bit X, bit bit_8, bit bit_0,
                                list<dag> pattern=[]> {
   def ""#x#suffix : MVE_VMLALDAVBase<

llvm/lib/Target/ARM/MCTargetDesc/ARMBaseInfo.h

enum {
     Xform16Bit    = 1 << 14,
 
     // ThumbArithFlagSetting - The instruction is a 16-bit flag setting Thumb
     // instruction. Used by the parser to determine whether to require the 'S'
     // suffix on the mnemonic (when not in an IT block) or preclude it (when
     // in an IT block).
     ThumbArithFlagSetting = 1 << 19,
 
-    // Whether an instruction can be included in an MVE tail-predicated loop.
+    // Whether an instruction can be included in an MVE tail-predicated loop,
+    // though extra validity checks may need to be performed too.
     ValidForTailPredication = 1 << 20,
 
     // Whether an instruction writes to the top/bottom half of a vector element
     // and leaves the other half untouched.
     RetainsPreviousHalfElement = 1 << 21,
 
     // Whether the instruction produces a scalar result from vector operands.
     HorizontalReduction = 1 << 22,

llvm/unittests/Target/ARM/MachineInstrTest.cpp

ASSERT_EQ(RetainsPreviousHalfElement(i), Valid)
               << ": mismatched expectation for tail-predicated safety\n";
   }
 }
 // Test for instructions that aren't immediately obviously valid within a
 // tail-predicated loop. This should be marked up in their tablegen
 // descriptions. Currently we, conservatively, disallow:
 // - cross beat carries.
 // - complex operations.
-// - horizontal operations.
+// - horizontal operations with exchange.
 // - byte swapping.
 // - interleaved memory instructions.
 // TODO: Add to this list once we can handle them safely.
 TEST(MachineInstrValidTailPredication, IsCorrect) {
 
   using namespace ARM;
 
   auto IsValidTPOpcode = [](unsigned Opcode) {
auto IsValidTPOpcode = [](unsigned Opcode) {
     case MVE_VMINAs32:
     case MVE_VMINAs8:
     case MVE_VMINs16:
     case MVE_VMINs32:
     case MVE_VMINs8:
     case MVE_VMINu16:
     case MVE_VMINu32:
     case MVE_VMINu8:
+    case MVE_VMLADAVas16:
+    case MVE_VMLADAVas32:
+    case MVE_VMLADAVas8:
+    case MVE_VMLADAVau16:
+    case MVE_VMLADAVau32:
+    case MVE_VMLADAVau8:
+    case MVE_VMLADAVs16:
+    case MVE_VMLADAVs32:
+    case MVE_VMLADAVs8:
+    case MVE_VMLADAVu16:
+    case MVE_VMLADAVu32:
+    case MVE_VMLADAVu8:
+    case MVE_VMLALDAVs16:
+    case MVE_VMLALDAVs32:
+    case MVE_VMLALDAVu16:
+    case MVE_VMLALDAVu32:
+    case MVE_VMLALDAVas16:
+    case MVE_VMLALDAVas32:
+    case MVE_VMLALDAVau16:
+    case MVE_VMLALDAVau32:
+    case MVE_VMLSDAVas16:
+    case MVE_VMLSDAVas32:
+    case MVE_VMLSDAVas8:
+    case MVE_VMLSDAVs16:
+    case MVE_VMLSDAVs32:
+    case MVE_VMLSDAVs8:
+    case MVE_VMLSLDAVas16:
+    case MVE_VMLSLDAVas32:
+    case MVE_VMLSLDAVs16:
+    case MVE_VMLSLDAVs32:
+    case MVE_VRMLALDAVHas32:
+    case MVE_VRMLALDAVHau32:
+    case MVE_VRMLALDAVHs32:
+    case MVE_VRMLALDAVHu32:
+    case MVE_VRMLSLDAVHas32:
+    case MVE_VRMLSLDAVHs32:
     case MVE_VMLAS_qr_s16:
     case MVE_VMLAS_qr_s32:
     case MVE_VMLAS_qr_s8:
     case MVE_VMLAS_qr_u16:
     case MVE_VMLAS_qr_u32:
     case MVE_VMLAS_qr_u8:
     case MVE_VMLA_qr_s16:
     case MVE_VMLA_qr_s32: