[AMDGPU] Remove -amdgpu-verify-regbanks-reassign

[foad]

Summary

There is a stall if two different sgpr operands come from the same bank,
unless they form an even-odd pair, in which case they are fetched
together and there is no stall even though they are in the same bank.

analyzeInst tries to count the number of stalls that an instruction
would have if all uses of Reg were replaced with a new register chosen
from Bank, but it can't do this accurately without knowing whether the
new register might form an even-odd pair with any of the other operands.

Because of this the CyclesSaved metric is not always accurate, so there
is no point in trying to verify it.

[foad]

Alternative solution #1: I think analyzeInst always returns the exact number of stalls or an overestimate, never an underestimate, so I could keep verifyCycles and change it to check the inequality StallCycles + CyclesSaved <= OriginalCycles.

Alternative solution #2: I could change tryReassign to scavenge a register from each bank before calling computeStallCycles, so we know exactly which register we're going to replace Reg with. But that might be expensive.

[rampitec]

I like the alternative solution #1.

[critson]

In D82643#2117280, @rampitec wrote:

I like the alternative solution #1.

I also agree with alt. solution #1.

To me a key aspect of the verifier is ensuring that forward progress is taking place.
For the bugs I was recently encountering this was true because CyclesSaved was going up, but StallCycles was not going down equivalently.
Because this was an infinite loop, CyclesSaved would keep growing indefinitely and the verifier would catch this.

[arsenm]

Is this still needed?

[arsenm]

Still needed?

llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp

 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/VirtRegMap.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/MathExtras.h"
 
 using namespace llvm;
 
-static cl::opt<unsigned> VerifyStallCycles("amdgpu-verify-regbanks-reassign",
-  cl::desc("Verify stall cycles in the regbanks reassign pass"),
-  cl::value_desc("0|1|2"),
-  cl::init(0), cl::Hidden);
-
 #define DEBUG_TYPE "amdgpu-regbanks-reassign"
 
 #define NUM_VGPR_BANKS 4
 #define NUM_SGPR_BANKS 8
 #define NUM_BANKS (NUM_VGPR_BANKS + NUM_SGPR_BANKS)
 #define SGPR_BANK_OFFSET NUM_VGPR_BANKS
 #define VGPR_BANK_MASK 0xf
 #define SGPR_BANK_MASK 0xff0
private:
 
   // Search for a register in Bank unused within LI.
   // Returns phys reg or NoRegister.
   unsigned scavengeReg(LiveInterval& LI, unsigned Bank) const;
 
   // Try to reassign candidate. Returns number or stall cycles saved.
   unsigned tryReassign(Candidate &C);
 
-  bool verifyCycles(MachineFunction &MF,
-                    unsigned OriginalCycles, unsigned CyclesSaved);
-
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 public:
   Printable printReg(unsigned Reg, unsigned SubReg = 0) const {
     return Printable([Reg, SubReg, this](raw_ostream &OS) {
       if (Register::isPhysicalRegister(Reg)) {
         OS << llvm::printReg(Reg, TRI);
         return;

 }
 
 void GCNRegBankReassign::removeCandidates(unsigned Reg) {
   Candidates.remove_if([Reg, this](const Candidate& C) {
     return C.MI->readsRegister(Reg, TRI);
   });
 }
 
-bool GCNRegBankReassign::verifyCycles(MachineFunction &MF,
-                                      unsigned OriginalCycles,
-                                      unsigned CyclesSaved) {
-  unsigned StallCycles = collectCandidates(MF, false);
-  LLVM_DEBUG(dbgs() << "=== After the pass " << StallCycles
-               << " stall cycles left\n");
-  return StallCycles + CyclesSaved == OriginalCycles;
-}
-
 bool GCNRegBankReassign::runOnMachineFunction(MachineFunction &MF) {
   ST = &MF.getSubtarget<GCNSubtarget>();
   if (!ST->hasRegisterBanking() || skipFunction(MF.getFunction()))
     return false;
 
   MRI = &MF.getRegInfo();
   TRI = ST->getRegisterInfo();
   MLI = &getAnalysis<MachineLoopInfo>();
LLVM_DEBUG(dbgs() << "\nCandidates:\n\n";
         dbgs() << "\n\n");
 
   unsigned CyclesSaved = 0;
   while (!Candidates.empty()) {
     Candidate C = Candidates.back();
     unsigned LocalCyclesSaved = tryReassign(C);
     CyclesSaved += LocalCyclesSaved;
 
-    if (VerifyStallCycles > 1 && !verifyCycles(MF, StallCycles, CyclesSaved))
-      report_fatal_error("RegBank reassign stall cycles verification failed.");
-
     Candidates.pop_back();
     if (LocalCyclesSaved) {
       removeCandidates(C.Reg);
       computeStallCycles(C.Reg, AMDGPU::NoRegister, -1, true);
       Candidates.sort();
 
       LLVM_DEBUG(dbgs() << "\nCandidates:\n\n";
             for (auto C : Candidates)
               C.dump(this);
             dbgs() << "\n\n");
     }
   }
   NumStallsRecovered += CyclesSaved;
 
   LLVM_DEBUG(dbgs() << "=== After the pass " << CyclesSaved
                << " cycles saved in function " << MF.getName() << '\n');
 
   Candidates.clear();
 
-  if (VerifyStallCycles == 1 && !verifyCycles(MF, StallCycles, CyclesSaved))
-    report_fatal_error("RegBank reassign stall cycles verification failed.");
-
   RegsUsed.clear();
 
   return CyclesSaved > 0;
 }