[PATCH] D82643: [AMDGPU] Remove -amdgpu-verify-regbanks-reassign

[Jay Foad via Phabricator via llvm-commits]

foad created this revision.
foad added reviewers: rampitec, arsenm.
Herald added subscribers: llvm-commits, kerbowa, hiraditya, t-tye, tpr, dstuttard, yaxunl, nhaehnle, wdng, jvesely, kzhuravl.
Herald added a project: LLVM.
foad added a comment.

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.


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82643

Files:
  llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp


Index: llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp
+++ llvm/lib/Target/AMDGPU/GCNRegBankReassign.cpp
@@ -48,11 +48,6 @@
 
 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
@@ -225,9 +220,6 @@
   // 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:
@@ -741,15 +733,6 @@
   });
 }
 
-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()))
@@ -795,9 +778,6 @@
     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);
@@ -817,9 +797,6 @@
 
   Candidates.clear();
 
-  if (VerifyStallCycles == 1 && !verifyCycles(MF, StallCycles, CyclesSaved))
-    report_fatal_error("RegBank reassign stall cycles verification failed.");
-
   RegsUsed.clear();
 
   return CyclesSaved > 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82643.273664.patch
Type: text/x-patch
Size: 2090 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200626/8ee8e26f/attachment.bin>