[ARM][MVE] VPT Blocks: findVCMPToFoldIntoVPS

This is a recommit of D71330, but with a few things fixed and changed:

1) ReachingDefAnalysis: this was not running with optnone as it was checking
skipFunction(), which other analysis passes don't do. I guess this is a
copy-paste from a codegen pass.
2) VPTBlockPass: here I've added skipFunction(), because like most/all
optimisations, we don't want to run this with optnone.

This fixes the issues with the initial/previous commit: the VPTBlockPass was
running with optnone, but ReachingDefAnalysis wasn't, and so VPTBlockPass was
crashing querying ReachingDefAnalysis.

I've added test case mve-vpt-block-optnone.mir to check that we don't run
VPTBlock with optnone.

Differential Revision: https://reviews.llvm.org/D71470

Author: Sjoerd Meijer

llvm/lib/CodeGen/ReachingDefAnalysis.cpp

@@ -133,8 +133,6 @@ void ReachingDefAnalysis::processBasicBlock(
 }
 
 bool ReachingDefAnalysis::runOnMachineFunction(MachineFunction &mf) {
-  if (skipFunction(mf.getFunction()))
-    return false;
   MF = &mf;
   TRI = MF->getSubtarget().getRegisterInfo();
 

llvm/lib/Target/ARM/MVEVPTBlockPass.cpp

@@ -22,9 +22,9 @@
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineInstrBundle.h"
 #include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/CodeGen/ReachingDefAnalysis.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/MC/MCInstrDesc.h"
-#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include <cassert>
 #include <new>
@@ -37,16 +37,21 @@ namespace {
   class MVEVPTBlock : public MachineFunctionPass {
   public:
     static char ID;
-    const Thumb2InstrInfo *TII;
-    const TargetRegisterInfo *TRI;
 
     MVEVPTBlock() : MachineFunctionPass(ID) {}
 
     bool runOnMachineFunction(MachineFunction &Fn) override;
 
+    void getAnalysisUsage(AnalysisUsage &AU) const override {
+      AU.setPreservesCFG();
+      AU.addRequired<ReachingDefAnalysis>();
+      MachineFunctionPass::getAnalysisUsage(AU);
+    }
+
     MachineFunctionProperties getRequiredProperties() const override {
       return MachineFunctionProperties().set(
-          MachineFunctionProperties::Property::NoVRegs);
+          MachineFunctionProperties::Property::NoVRegs).set(
+          MachineFunctionProperties::Property::TracksLiveness);
     }
 
     StringRef getPassName() const override {
@@ -55,6 +60,9 @@ namespace {
 
   private:
     bool InsertVPTBlocks(MachineBasicBlock &MBB);
+
+    const Thumb2InstrInfo *TII = nullptr;
+    ReachingDefAnalysis *RDA = nullptr;
   };
 
   char MVEVPTBlock::ID = 0;
@@ -63,41 +71,32 @@ namespace {
 
 INITIALIZE_PASS(MVEVPTBlock, DEBUG_TYPE, "ARM MVE VPT block pass", false, false)
 
-static MachineInstr *findVCMPToFoldIntoVPST(MachineBasicBlock::iterator MI,
-                                            const TargetRegisterInfo *TRI,
+static MachineInstr *findVCMPToFoldIntoVPST(MachineInstr *MI,
+                                            ReachingDefAnalysis *RDA,
                                             unsigned &NewOpcode) {
-  // Search backwards to the instruction that defines VPR. This may or not
-  // be a VCMP, we check that after this loop. If we find another instruction
-  // that reads cpsr, we return nullptr.
-  MachineBasicBlock::iterator CmpMI = MI;
-  while (CmpMI != MI->getParent()->begin()) {
-    --CmpMI;
-    if (CmpMI->modifiesRegister(ARM::VPR, TRI))
-      break;
-    if (CmpMI->readsRegister(ARM::VPR, TRI))
-      break;
-  }
-
-  if (CmpMI == MI)
-    return nullptr;
-  NewOpcode = VCMPOpcodeToVPT(CmpMI->getOpcode());
-  if (NewOpcode == 0)
+  // First, search backwards to the instruction that defines VPR
+  auto *Def = RDA->getReachingMIDef(MI, ARM::VPR);
+  if (!Def)
     return nullptr;
 
-  // Search forward from CmpMI to MI, checking if either register was def'd
-  if (registerDefinedBetween(CmpMI->getOperand(1).getReg(), std::next(CmpMI),
-                             MI, TRI))
+  // Now check that Def is a VCMP
+  if (!(NewOpcode = VCMPOpcodeToVPT(Def->getOpcode())))
     return nullptr;
-  if (registerDefinedBetween(CmpMI->getOperand(2).getReg(), std::next(CmpMI),
-                             MI, TRI))
+
+  // Check that Def's operands are not defined between the VCMP and MI, i.e.
+  // check that they have the same reaching def.
+  if (!RDA->hasSameReachingDef(Def, MI, Def->getOperand(1).getReg()) ||
+      !RDA->hasSameReachingDef(Def, MI, Def->getOperand(2).getReg()))
     return nullptr;
-  return &*CmpMI;
+
+  return Def;
 }
 
 bool MVEVPTBlock::InsertVPTBlocks(MachineBasicBlock &Block) {
   bool Modified = false;
   MachineBasicBlock::instr_iterator MBIter = Block.instr_begin();
   MachineBasicBlock::instr_iterator EndIter = Block.instr_end();
+  SmallVector<MachineInstr *, 4> RemovedVCMPs;
 
   while (MBIter != EndIter) {
     MachineInstr *MI = &*MBIter;
@@ -143,15 +142,19 @@ bool MVEVPTBlock::InsertVPTBlocks(MachineBasicBlock &Block) {
     // a VPST directly
     MachineInstrBuilder MIBuilder;
     unsigned NewOpcode;
-    MachineInstr *VCMP = findVCMPToFoldIntoVPST(MI, TRI, NewOpcode);
+    MachineInstr *VCMP = findVCMPToFoldIntoVPST(MI, RDA, NewOpcode);
     if (VCMP) {
       LLVM_DEBUG(dbgs() << "  folding VCMP into VPST: "; VCMP->dump());
       MIBuilder = BuildMI(Block, MI, dl, TII->get(NewOpcode));
       MIBuilder.addImm(BlockMask);
       MIBuilder.add(VCMP->getOperand(1));
       MIBuilder.add(VCMP->getOperand(2));
       MIBuilder.add(VCMP->getOperand(3));
-      VCMP->eraseFromParent();
+      // We delay removing the actual VCMP instruction by saving it to a list
+      // and deleting all instructions in this list in one go after we have
+      // created the VPT blocks. We do this in order not to invalidate the
+      // ReachingDefAnalysis that is queried by 'findVCMPToFoldIntoVPST'.
+      RemovedVCMPs.push_back(VCMP);
     } else {
       MIBuilder = BuildMI(Block, MI, dl, TII->get(ARM::MVE_VPST));
       MIBuilder.addImm(BlockMask);
@@ -162,18 +165,25 @@ bool MVEVPTBlock::InsertVPTBlocks(MachineBasicBlock &Block) {
 
     Modified = true;
   }
+
+  for (auto *I : RemovedVCMPs)
+    I->eraseFromParent();
+
   return Modified;
 }
 
 bool MVEVPTBlock::runOnMachineFunction(MachineFunction &Fn) {
+  if (skipFunction(Fn.getFunction()))
+    return false;
+
   const ARMSubtarget &STI =
       static_cast<const ARMSubtarget &>(Fn.getSubtarget());
 
   if (!STI.isThumb2() || !STI.hasMVEIntegerOps())
     return false;
 
   TII = static_cast<const Thumb2InstrInfo *>(STI.getInstrInfo());
-  TRI = STI.getRegisterInfo();
+  RDA = &getAnalysis<ReachingDefAnalysis>();
 
   LLVM_DEBUG(dbgs() << "********** ARM MVE VPT BLOCKS **********\n"
                     << "********** Function: " << Fn.getName() << '\n');

llvm/test/CodeGen/ARM/O3-pipeline.ll

@@ -144,6 +144,7 @@
 ; CHECK-NEXT:      Machine Natural Loop Construction
 ; CHECK-NEXT:      Machine Block Frequency Analysis
 ; CHECK-NEXT:      If Converter
+; CHECK-NEXT:      ReachingDefAnalysis
 ; CHECK-NEXT:      MVE VPT block insertion pass
 ; CHECK-NEXT:      Thumb IT blocks insertion pass
 ; CHECK-NEXT:      MachineDominator Tree Construction

llvm/test/CodeGen/Thumb2/mve-vpt-block-fold-vcmp.mir

@@ -0,0 +1,128 @@
+# RUN: llc -run-pass arm-mve-vpt %s -o - | FileCheck %s
+
+--- |
+  target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "thumbv8.1m.main-arm-unknown-eabihf"
+  define dso_local <4 x i32> @foo(<4 x i32>* %src, <4 x i32>* %src2, <4 x i32>* %src3, <4 x i32>* %dest, <4 x i32>* %dest2, <4 x i32>* %dest3, <4 x float> %a1) local_unnamed_addr #0 {
+  entry:
+    %c = fcmp one <4 x float> %a1, zeroinitializer
+    %w = call <4 x i32> @llvm.masked.load.v4i32.p0v4i32(<4 x i32>* %src, i32 4, <4 x i1> %c, <4 x i32> undef)
+    tail call void @llvm.masked.store.v4i32.p0v4i32(<4 x i32> %w, <4 x i32>* %dest, i32 4, <4 x i1> %c)
+    %w2 = call <4 x i32> @llvm.masked.load.v4i32.p0v4i32(<4 x i32>* %src2, i32 4, <4 x i1> %c, <4 x i32> undef)
+    tail call void @llvm.masked.store.v4i32.p0v4i32(<4 x i32> %w2, <4 x i32>* %dest2, i32 4, <4 x i1> %c)
+    %w3 = call <4 x i32> @llvm.masked.load.v4i32.p0v4i32(<4 x i32>* %src3, i32 4, <4 x i1> %c, <4 x i32> undef)
+    tail call void @llvm.masked.store.v4i32.p0v4i32(<4 x i32> %w3, <4 x i32>* %dest3, i32 4, <4 x i1> %c)
+    ret <4 x i32> %w3
+  }
+  declare <4 x i32> @llvm.masked.load.v4i32.p0v4i32(<4 x i32>*, i32 immarg, <4 x i1>, <4 x i32>) #2
+  declare void @llvm.masked.store.v4i32.p0v4i32(<4 x i32>, <4 x i32>*, i32 immarg, <4 x i1>) #3
+
+  attributes #0 = { nounwind readnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="128" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+armv8.1-m.main,+fp-armv8d16sp,+fp16,+fpregs,+fullfp16,+hwdiv,+lob,+mve.fp,+ras,+strict-align,+thumb-mode,+vfp2sp,+vfp3d16sp,+vfp4d16sp" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  attributes #1 = { nounwind readnone }
+  attributes #2 = { argmemonly nounwind readonly willreturn }
+  attributes #3 = { argmemonly nounwind willreturn }
+  attributes #4 = { noduplicate nounwind }
+  attributes #5 = { nounwind }
+
+  !llvm.module.flags = !{!0, !1}
+  !llvm.ident = !{!2}
+
+  !0 = !{i32 1, !"wchar_size", i32 4}
+  !1 = !{i32 1, !"min_enum_size", i32 4}
+  !2 = !{!"clang version 10.0.0 (http://github.com/llvm/llvm-project 90450197deaf91160a22825e6746d998aad05704)"}
+
+...
+---
+name:            foo
+alignment:       2
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+  - { reg: '$r1', virtual-reg: '' }
+  - { reg: '$r2', virtual-reg: '' }
+  - { reg: '$q0', virtual-reg: '' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       8
+  offsetAdjustment: 0
+  maxAlignment:    4
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+  - { id: 0, type: default, offset: 12, size: 4, alignment: 4, stack-id: default,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, type: default, offset: 8, size: 4, alignment: 8, stack-id: default,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 2, type: default, offset: 4, size: 4, alignment: 4, stack-id: default,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 3, type: default, offset: 0, size: 4, alignment: 8, stack-id: default,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -4, size: 4, alignment: 4,
+      stack-id: default, callee-saved-register: '$lr', callee-saved-restored: false,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, name: '', type: spill-slot, offset: -8, size: 4, alignment: 4,
+      stack-id: default, callee-saved-register: '$r7', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:       []
+constants:       []
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    liveins: $q0, $r0, $r1, $r2, $lr
+
+  ; CHECK:  BUNDLE implicit-def $vpr, implicit-def dead $q0, implicit-def $d0, implicit-def $s0, implicit-def $s1, implicit-def $d1, implicit-def $s2, implicit-def $s3, implicit $q0, implicit $zr, implicit killed $r0, implicit killed $r3, implicit killed $r1, implicit killed $lr {
+  ; CHECK:    MVE_VPTv4f32r 1, renamable $q0, $zr, 10, implicit-def $vpr
+  ; CHECK:    renamable $q0 = MVE_VLDRWU32 killed renamable $r0, 0, 1, internal renamable $vpr :: (load 16 from %ir.src, align 4)
+  ; CHECK:    MVE_VSTRWU32 internal killed renamable $q0, killed renamable $r3, 0, 1, internal renamable $vpr :: (store 16 into %ir.dest, align 4)
+  ; CHECK:    renamable $q0 = MVE_VLDRWU32 killed renamable $r1, 0, 1, internal renamable $vpr :: (load 16 from %ir.src2, align 4)
+  ; CHECK:    MVE_VSTRWU32 internal killed renamable $q0, killed renamable $lr, 0, 1, internal renamable $vpr :: (store 16 into %ir.dest2, align 4)
+  ; CHECK:  }
+  ; CHECK:  BUNDLE implicit-def $q0, implicit-def $d0, implicit-def $s0, implicit-def $s1, implicit-def $d1, implicit-def $s2, implicit-def $s3, implicit killed $vpr, implicit killed $r2, implicit killed $r12 {
+  ; CHECK:    MVE_VPST 4, implicit $vpr
+  ; CHECK:    renamable $q0 = MVE_VLDRWU32 killed renamable $r2, 0, 1, renamable $vpr :: (load 16 from %ir.src3, align 4)
+  ; CHECK:    MVE_VSTRWU32 internal renamable $q0, killed renamable $r12, 0, 1, killed renamable $vpr :: (store 16 into %ir.dest3, align 4)
+  ; CHECK:  }
+
+    $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r7, killed $lr
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r7, -8
+    $r7 = frame-setup tMOVr killed $sp, 14, $noreg
+    frame-setup CFI_INSTRUCTION def_cfa_register $r7
+    renamable $r12 = t2LDRi12 $r7, 16, 14, $noreg :: (load 4 from %fixed-stack.1)
+    renamable $lr = t2LDRi12 $r7, 12, 14, $noreg :: (load 4 from %fixed-stack.2)
+    renamable $r3 = t2LDRi12 $r7, 8, 14, $noreg :: (load 4 from %fixed-stack.3)
+    renamable $vpr = MVE_VCMPf32r renamable $q0, $zr, 10, 0, $noreg
+    renamable $q0 = MVE_VLDRWU32 killed renamable $r0, 0, 1, renamable $vpr :: (load 16 from %ir.src, align 4)
+    MVE_VSTRWU32 killed renamable $q0, killed renamable $r3, 0, 1, renamable $vpr :: (store 16 into %ir.dest, align 4)
+    renamable $q0 = MVE_VLDRWU32 killed renamable $r1, 0, 1, renamable $vpr :: (load 16 from %ir.src2, align 4)
+    MVE_VSTRWU32 killed renamable $q0, killed renamable $lr, 0, 1, renamable $vpr :: (store 16 into %ir.dest2, align 4)
+    renamable $q0 = MVE_VLDRWU32 killed renamable $r2, 0, 1, renamable $vpr :: (load 16 from %ir.src3, align 4)
+    MVE_VSTRWU32 renamable $q0, killed renamable $r12, 0, 1, killed renamable $vpr :: (store 16 into %ir.dest3, align 4)
+    $sp = t2LDMIA_RET $sp, 14, $noreg, def $r7, def $pc, implicit $q0
+
+...

llvm/test/CodeGen/Thumb2/mve-vpt-block-optnone.mir

@@ -0,0 +1,75 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -run-pass arm-mve-vpt %s -o - | FileCheck %s
+
+--- |
+  target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "thumbv8.1m.main-arm-none-eabi"
+
+  define hidden arm_aapcs_vfpcc <4 x float> @test_vminnmq_m_f32_v2(<4 x float> %inactive, <4 x float> %a, <4 x float> %b, i16 zeroext %p) local_unnamed_addr #0 {
+  entry:
+    %conv.i = zext i16 %p to i32
+    %0 = tail call nnan ninf nsz <4 x float> @llvm.arm.mve.vminnm.m.v4f32.v4f32.v4f32.v4f32.i32(<4 x float> %inactive, <4 x float> %a, <4 x float> %b, i32 %conv.i) #2
+    ret <4 x float> %0
+  }
+
+  declare <4 x float> @llvm.arm.mve.vminnm.m.v4f32.v4f32.v4f32.v4f32.i32(<4 x float>, <4 x float>, <4 x float>, i32) #1
+
+  attributes #0 = { noinline optnone nounwind readnone "correctly-rounded-divide-sqrt-fp-math"="false" "denormal-fp-math"="preserve-sign" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="128" "no-frame-pointer-elim"="false" "no-infs-fp-math"="true" "no-jump-tables"="false" "no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+armv8.1-m.main,+hwdiv,+mve.fp,+ras,+thumb-mode" "unsafe-fp-math"="false" "use-soft-float"="false" }
+  attributes #1 = { nounwind readnone }
+  attributes #2 = { nounwind }
+
+
+...
+---
+name:            test_vminnmq_m_f32_v2
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+failedISel:      false
+tracksRegLiveness: true
+hasWinCFI:       false
+registers:       []
+liveins:
+  - { reg: '$q0', virtual-reg: '' }
+  - { reg: '$q1', virtual-reg: '' }
+  - { reg: '$q2', virtual-reg: '' }
+  - { reg: '$r0', virtual-reg: '' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    0
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 0
+  cvBytesOfCalleeSavedRegisters: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  localFrameSize:  0
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:      []
+stack:           []
+constants:       []
+body:             |
+  bb.0.entry:
+    liveins: $q0, $q1, $q2, $r0
+
+    ; CHECK-LABEL: name: test_vminnmq_m_f32_v2
+    ; CHECK: liveins: $q0, $q1, $q2, $r0
+    ; CHECK: $vpr = VMSR_P0 killed $r0, 14, $noreg
+    ; CHECK: renamable $q0 = nnan ninf nsz MVE_VMINNMf32 killed renamable $q1, killed renamable $q2, 1, killed renamable $vpr, killed renamable $q0
+    ; CHECK: tBX_RET 14, $noreg, implicit $q0
+
+      $vpr = VMSR_P0 killed $r0, 14, $noreg
+    renamable $q0 = nnan ninf nsz MVE_VMINNMf32 killed renamable $q1, killed renamable $q2, 1, killed renamable $vpr, killed renamable $q0
+    tBX_RET 14, $noreg, implicit $q0
+
+...