[ARM] Add more validForTailPredication

[samparker]

Summary

Modify the unit test to inspect all MVE instructions and mark the load/store/move of vpr/p0 as valid, as well as the remaining scalar shifts.

[SjoerdMeijer]

Looks reasonable

[dmgreen]

llvm/lib/Target/ARM/ARMInstrVFP.td

2494

Can you explain what ramifications making these validForTailPredication has? And why it helps to make these part of the MVEDomain? (The are really just VFP instructions. They are unpredictable for non-mve, but given the options I think I would move these and scalar shifts out of the MVE domain!)

They sound like they would need to be handled specially in the backend pass anyway, but that pass is a bit complex for me to follow at this point.

[samparker]

llvm/lib/Target/ARM/ARMInstrVFP.td

2494

It makes checking for instructions easier, as we ignore any non-mve instructions as they shouldn't affect anything with tail predication. These load/store/move can have an affect because they access VPR/P0. All these require MVE so I'm not sure I understand why you'd prefer them out of the MVEDomain?

[dmgreen]

llvm/lib/Target/ARM/ARMInstrVFP.td

2494

It is useful to have a list of instructions that are all the beatwise MVE instructions. We use it for that (downstream) in scheduling, which will need an adjustment for this. We essentially get to choose what goes into the domain (as far as I understand), so treating it for just the set of instructions that work like MVE instructions sounds more useful to me.

The ARMLowOverheadLoops pass presumably has to look at VFP instructions anyway (and anything that could touch vfp regs).

[samparker]

llvm/lib/Target/ARM/ARMInstrVFP.td

2494

This sounds fair enough, maybe MVEBeatDomain or something should be used for clarity. I can have a helper which looks for this domain and/or vpr operands. We don't explicitly track vfp regs, but they should be considered as use/defs of their q-reg. Are there are instructions that you are particularly concerned with?

[dmgreen]

llvm/lib/Target/ARM/ARMInstrVFP.td

2494

I was thinking of anything that crossed lanes. like vmov s7, s0. I think a reverse load at the moment (if it got past cost modelling) would be something like, but you could imagine something similar in a lot of other cases too, if they were written from intrinsics.

llvm/lib/Target/ARM/ARMInstrMVE.td

class MVE_VMOV_lane_base<dag oops, dag iops, InstrItinClass itin, string asm,
   let DecoderNamespace = "MVE";
 }
 
 class MVE_ScalarShift<string iname, dag oops, dag iops, string asm, string cstr,
             list<dag> pattern=[]>
   : MVE_MI_with_pred<oops, iops, NoItinerary, iname, asm, cstr, pattern> {
   let Inst{31-20} = 0b111010100101;
   let Inst{8} = 0b1;
-
+  let validForTailPredication=1;
 }
 
 class MVE_ScalarShiftSingleReg<string iname, dag iops, string asm, string cstr,
                     list<dag> pattern=[]>
   : MVE_ScalarShift<iname, (outs rGPR:$RdaDest), iops, asm, cstr, pattern> {
   bits<4> RdaDest;
 
   let Inst{19-16} = RdaDest{3-0};

llvm/lib/Target/ARM/ARMInstrVFP.td

def VMRS_FPCXTNS : MovFromVFP<0b1110 /* fpcxtns */, (outs GPR:$Rt), (ins),
                                  "vmrs", "\t$Rt, fpcxtns", []>;
  }
  let Predicates = [HasV8_1MMainline, Has8MSecExt] in {
    // System level FPSCR -> GPR, with context saving for security extensions
    def VMRS_FPCXTS : MovFromVFP<0b1111 /* fpcxts */, (outs GPR:$Rt), (ins),
                                 "vmrs", "\t$Rt, fpcxts", []>;
  }
 
- let Predicates = [HasV8_1MMainline, HasMVEInt] in {
+ let Predicates = [HasV8_1MMainline, HasMVEInt],
+                   D=MVEDomain, validForTailPredication=1 in {
    // System level VPR/P0 -> GPR
    let Uses = [VPR] in
    def VMRS_VPR : MovFromVFP<0b1100 /* vpr */, (outs GPR:$Rt), (ins),
                              "vmrs", "\t$Rt, vpr", []>;
 
    def VMRS_P0  : MovFromVFP<0b1101 /* p0 */, (outs GPR:$Rt), (ins VCCR:$cond),
                              "vmrs", "\t$Rt, p0", []>;
  }
let Defs = [FPSCR] in {
   defm VSTR_FPSCR_NZCVQC   : vfp_vstrldr_sysreg<0b0,0b0010, "fpscr_nzcvqc">;
 
   let Predicates = [HasV8_1MMainline, Has8MSecExt] in {
     defm VSTR_FPCXTNS      : vfp_vstrldr_sysreg<0b0,0b1110, "fpcxtns">;
     defm VSTR_FPCXTS       : vfp_vstrldr_sysreg<0b0,0b1111, "fpcxts">;
   }
 }
 
-let Predicates = [HasV8_1MMainline, HasMVEInt] in {
+let Predicates = [HasV8_1MMainline, HasMVEInt],
+                  D=MVEDomain, validForTailPredication=1 in {
   let Uses = [VPR] in {
     defm VSTR_VPR          : vfp_vstrldr_sysreg<0b0,0b1100, "vpr">;
   }
   defm VSTR_P0             : vfp_vstrldr_sysreg<0b0,0b1101, "p0",
                                                 (outs), (ins VCCR:$P0)>;
+
+  let Defs = [VPR] in {
+    defm VLDR_VPR          : vfp_vstrldr_sysreg<0b1,0b1100, "vpr">;
+  }
+  defm VLDR_P0             : vfp_vstrldr_sysreg<0b1,0b1101, "p0",
+                                                (outs VCCR:$P0), (ins)>;
 }
 
 let Uses = [FPSCR] in {
   defm VLDR_FPSCR          : vfp_vstrldr_sysreg<0b1,0b0001, "fpscr">;
   defm VLDR_FPSCR_NZCVQC   : vfp_vstrldr_sysreg<0b1,0b0010, "fpscr_nzcvqc">;
 
   let Predicates = [HasV8_1MMainline, Has8MSecExt] in {
     defm VLDR_FPCXTNS      : vfp_vstrldr_sysreg<0b1,0b1110, "fpcxtns">;
     defm VLDR_FPCXTS       : vfp_vstrldr_sysreg<0b1,0b1111, "fpcxts">;
   }
 }
-
-let Predicates = [HasV8_1MMainline, HasMVEInt] in {
-  let Defs = [VPR] in {
-    defm VLDR_VPR          : vfp_vstrldr_sysreg<0b1,0b1100, "vpr">;
-  }
-  defm VLDR_P0             : vfp_vstrldr_sysreg<0b1,0b1101, "p0",
-                                                (outs VCCR:$P0), (ins)>;
-}

llvm/unittests/Target/ARM/MachineInstrTest.cpp

TEST(MachineInstrValidTailPredication, IsCorrect) {
 
   auto IsValidTPOpcode = [](unsigned Opcode) {
     switch (Opcode) {
     default:
       return false;
     case MVE_ASRLi:
     case MVE_ASRLr:
     case MVE_LSRL:
+    case MVE_LSLLi:
+    case MVE_LSLLr:
     case MVE_SQRSHR:
+    case MVE_SQRSHRL:
     case MVE_SQSHL:
+    case MVE_SQSHLL:
     case MVE_SRSHR:
+    case MVE_SRSHRL:
     case MVE_UQRSHL:
+    case MVE_UQRSHLL:
     case MVE_UQSHL:
+    case MVE_UQSHLL:
     case MVE_URSHR:
+    case MVE_URSHRL:
     case MVE_VABDf16:
     case MVE_VABDf32:
     case MVE_VABDs16:
     case MVE_VABDs32:
     case MVE_VABDs8:
     case MVE_VABDu16:
     case MVE_VABDu32:
     case MVE_VABDu8:
auto IsValidTPOpcode = [](unsigned Opcode) {
     case MVE_VSUB_qr_i16:
     case MVE_VSUB_qr_i32:
     case MVE_VSUB_qr_i8:
     case MVE_VSUBf16:
     case MVE_VSUBf32:
     case MVE_VSUBi16:
     case MVE_VSUBi32:
     case MVE_VSUBi8:
+    case VLDR_P0_off:
+    case VLDR_P0_post:
+    case VLDR_P0_pre:
+    case VLDR_VPR_off:
+    case VLDR_VPR_post:
+    case VLDR_VPR_pre:
+    case VSTR_P0_off:
+    case VSTR_P0_post:
+    case VSTR_P0_pre:
+    case VSTR_VPR_off:
+    case VSTR_VPR_post:
+    case VSTR_VPR_pre:
+    case VMRS_P0:
+    case VMRS_VPR:
       return true;
     }
   };
 
   LLVMInitializeARMTargetInfo();
   LLVMInitializeARMTarget();
   LLVMInitializeARMTargetMC();
 
   auto TT(Triple::normalize("thumbv8.1m.main-none-none-eabi"));
   std::string Error;
   const Target *T = TargetRegistry::lookupTarget(TT, Error);
   if (!T) {
     dbgs() << Error;
     return;
   }
 
   TargetOptions Options;
   auto TM = std::unique_ptr<LLVMTargetMachine>(
     static_cast<LLVMTargetMachine*>(
       T->createTargetMachine(TT, "generic", "", Options, None, None,
                              CodeGenOpt::Default)));
   ARMSubtarget ST(TM->getTargetTriple(), std::string(TM->getTargetCPU()),
                   std::string(TM->getTargetFeatureString()),
                   *static_cast<const ARMBaseTargetMachine *>(TM.get()), false);
-  const ARMBaseInstrInfo *TII = ST.getInstrInfo();
-  auto MII = TM->getMCInstrInfo();
 
+  auto MII = TM->getMCInstrInfo();
   for (unsigned i = 0; i < ARM::INSTRUCTION_LIST_END; ++i) {
-    const MCInstrDesc &Desc = TII->get(i);
-
-    for (auto &Op : Desc.operands()) {
-      // Only check instructions that access the MQPR regs.
-      if ((Op.OperandType & MCOI::OPERAND_REGISTER) == 0 ||
-          (Op.RegClass != ARM::MQPRRegClassID &&
-           Op.RegClass != ARM::QQPRRegClassID &&
-           Op.RegClass != ARM::QQQQPRRegClassID))
-        continue;
-
-      uint64_t Flags = MII->get(i).TSFlags;
-      bool Valid = (Flags & ARMII::ValidForTailPredication) != 0;
-      ASSERT_EQ(IsValidTPOpcode(i), Valid)
-                << MII->getName(i)
-                << ": mismatched expectation for tail-predicated safety\n";
-      break;
-    }
+    if ((Flags & ARMII::DomainMask) != ARMII::DomainMVE)
+      continue;
   }
 }
 
 TEST(MachineInstr, HasSideEffects) {
   using namespace ARM;
   std::set<unsigned> UnpredictableOpcodes = {
       // MVE Instructions
       MVE_VCTP8,