[ARM] Remove MVEDomain from VLDR/STR of P0

[samparker]

Summary

Remove the domain from the instructions and create a shouldInspect helper for LowOverheadLoops which queries it or a vpr operand.

[dmgreen]

Thanks for doing this.

llvm/lib/Target/ARM/ARMInstrVFP.td

2863

What would be the ramification of leaving this? I see VMRS_P0 is no longer validForTailPredication.

Would there be anything that stopped the code from spilling in the loop and reloading outside of it? Would the stored predicate always end up being the same?

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp

79

Would this match VSTR_P0? (Or, would they otherwise be handled somewhere?)

[samparker]

• All the VFP instructions now have had validForTailPredication the DomainMVE removed.
• LowOverheadLoops will inspect any instructions that are in the DomainMVE, or if they use/def the VPR. Helpers for each are added.
• LowOverheadLoops continues to reject any use of the VPR that isn't a predicate operand.
• Rebased after adding some more tests for P0 usage.

[dmgreen]

Thanks. LGTM from what I understand.

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],
-                   D=MVEDomain, validForTailPredication=1 in {
+ let Predicates = [HasV8_1MMainline, HasMVEInt] 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],
-                  D=MVEDomain, validForTailPredication=1 in {
+let Predicates = [HasV8_1MMainline, HasMVEInt] 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">;

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp

 #include "llvm/CodeGen/ReachingDefAnalysis.h"
 #include "llvm/MC/MCInstrDesc.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "arm-low-overhead-loops"
 #define ARM_LOW_OVERHEAD_LOOPS_NAME "ARM Low Overhead Loops pass"
 
+static bool isVectorPredicated(MachineInstr *MI) {
+  int PIdx = llvm::findFirstVPTPredOperandIdx(*MI);
+  return PIdx != -1 && MI->getOperand(PIdx + 1).getReg() == ARM::VPR;
+}
+
+static bool isVectorPredicate(MachineInstr *MI) {
+  return MI->findRegisterDefOperandIdx(ARM::VPR) != -1;
+}
+
+static bool hasVPRUse(MachineInstr *MI) {
+  return MI->findRegisterUseOperandIdx(ARM::VPR) != -1;
+}
+
+static bool isDomainMVE(MachineInstr *MI) {
+  uint64_t Domain = MI->getDesc().TSFlags & ARMII::DomainMask;
+  return Domain == ARMII::DomainMVE;
+}
+
+static bool shouldInspect(MachineInstr &MI) {
+  return isDomainMVE(&MI) || isVectorPredicate(&MI) ||
+    hasVPRUse(&MI);
+}
+
 namespace {
 
   using InstSet = SmallPtrSetImpl<MachineInstr *>;
 
   class PostOrderLoopTraversal {
     MachineLoop &ML;
     MachineLoopInfo &MLI;
     SmallPtrSet<MachineBasicBlock*, 4> Visited;
if (RDA.isSafeToRemove(Def, ElementChain, Ignore)) {
                  for (auto *MI : ElementChain)
                    dbgs() << " - " << *MI);
       ToRemove.insert(ElementChain.begin(), ElementChain.end());
     }
   }
   return true;
 }
 
-static bool isVectorPredicated(MachineInstr *MI) {
-  int PIdx = llvm::findFirstVPTPredOperandIdx(*MI);
-  return PIdx != -1 && MI->getOperand(PIdx + 1).getReg() == ARM::VPR;
-}
-
 static bool isRegInClass(const MachineOperand &MO,
                          const TargetRegisterClass *Class) {
   return MO.isReg() && MO.getReg() && Class->contains(MO.getReg());
 }
 
 // MVE 'narrowing' operate on half a lane, reading from half and writing
 // to half, which are referred to has the top and bottom half. The other
 // half retains its previous value.
bool LowOverheadLoop::ValidateLiveOuts() {
   // set and build from there.
   const TargetRegisterClass *QPRs = TRI.getRegClass(ARM::MQPRRegClassID);
   SetVector<MachineInstr *> FalseLanesUnknown;
   SmallPtrSet<MachineInstr *, 4> FalseLanesZero;
   SmallPtrSet<MachineInstr *, 4> Predicated;
   MachineBasicBlock *Header = ML.getHeader();
 
   for (auto &MI : *Header) {
-    const MCInstrDesc &MCID = MI.getDesc();
-    uint64_t Flags = MCID.TSFlags;
-    if ((Flags & ARMII::DomainMask) != ARMII::DomainMVE)
+    if (!shouldInspect(MI))
       continue;
 
     if (isVCTP(&MI) || isVPTOpcode(MI.getOpcode()))
       continue;
 
     bool isPredicated = isVectorPredicated(&MI);
     bool retainsOrReduces =
       retainsPreviousHalfElement(MI) || isHorizontalReduction(MI);
void LowOverheadLoop::CheckLegality(ARMBasicBlockUtils *BBUtils) {
   LLVM_DEBUG(if (CannotTailPredicate)
              dbgs() << "ARM Loops: Couldn't validate tail predicate.\n");
 }
 
 bool LowOverheadLoop::ValidateMVEInst(MachineInstr* MI) {
   if (CannotTailPredicate)
     return false;
 
-  const MCInstrDesc &MCID = MI->getDesc();
-  uint64_t Flags = MCID.TSFlags;
-  if ((Flags & ARMII::DomainMask) != ARMII::DomainMVE)
+  if (!shouldInspect(*MI))
     return true;
 
   if (MI->getOpcode() == ARM::MVE_VPSEL ||
       MI->getOpcode() == ARM::MVE_VPNOT) {
     // TODO: Allow VPSEL and VPNOT, we currently cannot because:
     // 1) It will use the VPR as a predicate operand, but doesn't have to be
     //    instead a VPT block, which means we can assert while building up
     //    the VPT block because we don't find another VPT or VPST to being a new
if (MI->getOpcode() != ARM::MVE_VPST) {
       CurrentPredicate.insert(MI);
     }
 
     VPTBlocks.emplace_back(MI, CurrentPredicate);
     CurrentBlock = &VPTBlocks.back();
     return true;
   }
 
+  // Inspect uses first so that any instructions that alter the VPR don't
+  // alter the predicate upon themselves.
+  const MCInstrDesc &MCID = MI->getDesc();
   bool IsUse = false;
   bool IsDef = false;
   for (int i = MI->getNumOperands() - 1; i >= 0; --i) {
     const MachineOperand &MO = MI->getOperand(i);
     if (!MO.isReg() || MO.getReg() != ARM::VPR)
       continue;
 
     if (MO.isDef()) {
bool LowOverheadLoop::ValidateMVEInst(MachineInstr* MI) {
   if (IsDef && !IsUse && VCTP && !isVCTP(MI)) {
     LLVM_DEBUG(dbgs() << "ARM Loops: Found disjoint vpr def: " << *MI);
     return false;
   }
 
   // If we find an instruction that has been marked as not valid for tail
   // predication, only allow the instruction if it's contained within a valid
   // VPT block.
-  if ((Flags & ARMII::ValidForTailPredication) == 0) {
-    LLVM_DEBUG(dbgs() << "ARM Loops: Can't tail predicate: " << *MI);
+  bool RequiresExplicitPredication =
+    (MCID.TSFlags & ARMII::ValidForTailPredication) == 0;
+  if (isDomainMVE(MI) && RequiresExplicitPredication) {
+    LLVM_DEBUG(if (!IsUse)
+               dbgs() << "ARM Loops: Can't tail predicate: " << *MI);
     return IsUse;
   }
 
   // If the instruction is already explicitly predicated, then the conversion
   // will be fine, but ensure that all store operations are predicated.
   return !IsUse && MI->mayStore() ? false : true;
 }
 

llvm/test/CodeGen/Thumb2/LowOverheadLoops/vctp-in-vpt.mir

body:             |
   ; CHECK:   frame-setup CFI_INSTRUCTION offset $r7, -8
   ; CHECK:   dead $r7 = frame-setup tMOVr $sp, 14 /* CC::al */, $noreg
   ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_register $r7
   ; CHECK:   $sp = frame-setup tSUBspi $sp, 1, 14 /* CC::al */, $noreg
   ; CHECK:   tCBZ $r2, %bb.3
   ; CHECK: bb.1.bb3:
   ; CHECK:   successors: %bb.2(0x80000000)
   ; CHECK:   liveins: $r0, $r1, $r2, $r3
+  ; CHECK:   renamable $r12 = t2ADDri renamable $r2, 3, 14 /* CC::al */, $noreg, $noreg
+  ; CHECK:   renamable $lr = t2MOVi 1, 14 /* CC::al */, $noreg, $noreg
+  ; CHECK:   renamable $r12 = t2BICri killed renamable $r12, 3, 14 /* CC::al */, $noreg, $noreg
   ; CHECK:   $vpr = VMSR_P0 killed $r3, 14 /* CC::al */, $noreg
+  ; CHECK:   renamable $r12 = t2SUBri killed renamable $r12, 4, 14 /* CC::al */, $noreg, $noreg
   ; CHECK:   VSTR_P0_off killed renamable $vpr, $sp, 0, 14 /* CC::al */, $noreg :: (store 4 into %stack.0)
   ; CHECK:   $r3 = tMOVr $r0, 14 /* CC::al */, $noreg
-  ; CHECK:   $lr = MVE_DLSTP_32 killed renamable $r2
+  ; CHECK:   renamable $lr = nuw nsw t2ADDrs killed renamable $lr, killed renamable $r12, 19, 14 /* CC::al */, $noreg, $noreg
+  ; CHECK:   $lr = t2DLS killed renamable $lr
   ; CHECK: bb.2.bb9:
   ; CHECK:   successors: %bb.2(0x7c000000), %bb.3(0x04000000)
-  ; CHECK:   liveins: $lr, $r0, $r1, $r3
+  ; CHECK:   liveins: $lr, $r0, $r1, $r2, $r3
   ; CHECK:   renamable $vpr = VLDR_P0_off $sp, 0, 14 /* CC::al */, $noreg :: (load 4 from %stack.0)
-  ; CHECK:   MVE_VPST 4, implicit $vpr
+  ; CHECK:   MVE_VPST 2, implicit $vpr
+  ; CHECK:   renamable $vpr = MVE_VCTP32 renamable $r2, 1, killed renamable $vpr
   ; CHECK:   renamable $r1, renamable $q0 = MVE_VLDRWU32_post killed renamable $r1, 16, 1, renamable $vpr
   ; CHECK:   renamable $r3, renamable $q1 = MVE_VLDRWU32_post killed renamable $r3, 16, 1, killed renamable $vpr
   ; CHECK:   $vpr = VMSR_P0 $r3, 14 /* CC::al */, $noreg
+  ; CHECK:   renamable $r2, dead $cpsr = tSUBi8 killed renamable $r2, 4, 14 /* CC::al */, $noreg
   ; CHECK:   renamable $q0 = nsw MVE_VMULi32 killed renamable $q1, killed renamable $q0, 0, $noreg, undef renamable $q0
   ; CHECK:   MVE_VPST 8, implicit $vpr
   ; CHECK:   MVE_VSTRWU32 killed renamable $q0, killed renamable $r0, 0, 1, killed renamable $vpr
   ; CHECK:   $r0 = tMOVr $r3, 14 /* CC::al */, $noreg
-  ; CHECK:   $lr = MVE_LETP killed renamable $lr, %bb.2
+  ; CHECK:   $lr = t2LEUpdate killed renamable $lr, %bb.2
   ; CHECK: bb.3.bb27:
   ; CHECK:   $sp = tADDspi $sp, 1, 14 /* CC::al */, $noreg
   ; CHECK:   tPOP_RET 14 /* CC::al */, $noreg, def $r7, def $pc
   bb.0.bb:
     successors: %bb.3(0x30000000), %bb.1(0x50000000)
     liveins: $r0, $r1, $r2, $r3, $lr
 
     frame-setup tPUSH 14, $noreg, killed $lr, implicit-def $sp, implicit $sp