[MIPS] Check if register is non-null before calling isSuperOrSubRegisterEq (NFCI)

[barannikov88]

Summary

D151036 adds an assertions that prohibits iterating over sub- and
super-registers of a null register. This is already the case when
iterating over register units of a null register, and worked by
accident for sub- and super-registers.

[barannikov88]

Rebase onto 8e16365c to make the bot green again

[wzssyqa]

I noticed there are some lines like:

2955     unsigned TmpReg = DstReg;
2956     if (UseSrcReg &&
2957         getContext().getRegisterInfo()->isSuperOrSubRegisterEq(DstReg,
2958                                                                SrcReg)) {
2959       // If $rs is the same as $rd, we need to use AT.
2960       // If it is not available we exit.
2961       unsigned ATReg = getATReg(IDLoc);
2962       if (!ATReg)
2963         return true;
2964       TmpReg = ATReg;
2965     }

Is UseSrcReg OK for here?

[barannikov88]

In D151288#4367391, @wzssyqa wrote:

I noticed there are some lines like:

2955     unsigned TmpReg = DstReg;
2956     if (UseSrcReg &&
2957         getContext().getRegisterInfo()->isSuperOrSubRegisterEq(DstReg,
2958                                                                SrcReg)) {
2959       // If $rs is the same as $rd, we need to use AT.
2960       // If it is not available we exit.
2961       unsigned ATReg = getATReg(IDLoc);
2962       if (!ATReg)
2963         return true;
2964       TmpReg = ATReg;
2965     }

Is UseSrcReg OK for here?

I honestly don't know :) Sounds reasonable, let's try it

[barannikov88]

Check UseSrcReg instead of SrcReg and rebase onto 8e16365c to make the bots happy

[wzssyqa]

I agree with this patch, while I have no approval permission.

@MaskRay

[barannikov88]

@wzssyqa Thanks for the quick review!

[barannikov88]

Whoops, I committed a change with wrong commit message referring to this review (copy&pasted) and the review was closed automatically >_<
Reopening

[barannikov88]

Revert to the correct revision

llvm/lib/CodeGen/AggressiveAntiDepBreaker.cpp

void AggressiveAntiDepBreaker::Observe(MachineInstr &MI, unsigned Count,
   PrescanInstruction(MI, Count, PassthruRegs);
   ScanInstruction(MI, Count);
 
   LLVM_DEBUG(dbgs() << "Observe: ");
   LLVM_DEBUG(MI.dump());
   LLVM_DEBUG(dbgs() << "\tRegs:");
 
   std::vector<unsigned> &DefIndices = State->GetDefIndices();
-  for (unsigned Reg = 0; Reg != TRI->getNumRegs(); ++Reg) {
+  for (unsigned Reg = 1; Reg != TRI->getNumRegs(); ++Reg) {
     // If Reg is current live, then mark that it can't be renamed as
     // we don't know the extent of its live-range anymore (now that it
     // has been scheduled). If it is not live but was defined in the
     // previous schedule region, then set its def index to the most
     // conservative location (i.e. the beginning of the previous
     // schedule region).
     if (State->IsLive(Reg)) {
       LLVM_DEBUG(if (State->GetGroup(Reg) != 0) dbgs()
if (CriticalPathSet.any()) {
     }
     assert(CriticalPathSU && "Failed to find SUnit critical path");
     CriticalPathMI = CriticalPathSU->getInstr();
   }
 
 #ifndef NDEBUG
   LLVM_DEBUG(dbgs() << "\n===== Aggressive anti-dependency breaking\n");
   LLVM_DEBUG(dbgs() << "Available regs:");
-  for (unsigned Reg = 0; Reg < TRI->getNumRegs(); ++Reg) {
+  for (unsigned Reg = 1; Reg < TRI->getNumRegs(); ++Reg) {
     if (!State->IsLive(Reg))
       LLVM_DEBUG(dbgs() << " " << printReg(Reg, TRI));
   }
   LLVM_DEBUG(dbgs() << '\n');
 #endif
 
   BitVector RegAliases(TRI->getNumRegs());
 

llvm/lib/CodeGen/CriticalAntiDepBreaker.cpp

: MF(MFi), MRI(MF.getRegInfo()), TII(MF.getSubtarget().getInstrInfo()),
       TRI(MF.getSubtarget().getRegisterInfo()), RegClassInfo(RCI),
       Classes(TRI->getNumRegs(), nullptr), KillIndices(TRI->getNumRegs(), 0),
       DefIndices(TRI->getNumRegs(), 0), KeepRegs(TRI->getNumRegs(), false) {}
 
 CriticalAntiDepBreaker::~CriticalAntiDepBreaker() = default;
 
 void CriticalAntiDepBreaker::StartBlock(MachineBasicBlock *BB) {
   const unsigned BBSize = BB->size();
-  for (unsigned i = 0, e = TRI->getNumRegs(); i != e; ++i) {
+  for (unsigned i = 1, e = TRI->getNumRegs(); i != e; ++i) {
     // Clear out the register class data.
     Classes[i] = nullptr;
 
     // Initialize the indices to indicate that no registers are live.
     KillIndices[i] = ~0u;
     DefIndices[i] = BBSize;
   }
 
void CriticalAntiDepBreaker::Observe(MachineInstr &MI, unsigned Count,
 
   // FIXME: It may be possible to remove the isKill() restriction once PR18663
   // has been properly fixed. There can be value in processing kills as seen in
   // the AggressiveAntiDepBreaker class.
   if (MI.isDebugInstr() || MI.isKill())
     return;
   assert(Count < InsertPosIndex && "Instruction index out of expected range!");
 
-  for (unsigned Reg = 0; Reg != TRI->getNumRegs(); ++Reg) {
+  for (unsigned Reg = 1; Reg != TRI->getNumRegs(); ++Reg) {
     if (KillIndices[Reg] != ~0u) {
       // If Reg is currently live, then mark that it can't be renamed as
       // we don't know the extent of its live-range anymore (now that it
       // has been scheduled).
       Classes[Reg] = reinterpret_cast<TargetRegisterClass *>(-1);
       KillIndices[Reg] = Count;
     } else if (DefIndices[Reg] < InsertPosIndex && DefIndices[Reg] >= Count) {
       // Any register which was defined within the previous scheduling region
for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
       MachineOperand &MO = MI.getOperand(i);
 
       if (MO.isRegMask()) {
         auto ClobbersPhysRegAndSubRegs = [&](unsigned PhysReg) {
           return all_of(TRI->subregs_inclusive(PhysReg),
                         [&](MCPhysReg SR) { return MO.clobbersPhysReg(SR); });
         };
 
-        for (unsigned i = 0, e = TRI->getNumRegs(); i != e; ++i) {
+        for (unsigned i = 1, e = TRI->getNumRegs(); i != e; ++i) {
           if (ClobbersPhysRegAndSubRegs(i)) {
             DefIndices[i] = Count;
             KillIndices[i] = ~0u;
             KeepRegs.reset(i);
             Classes[i] = nullptr;
             RegRefs.erase(i);
           }
         }
BreakAntiDependencies(const std::vector<SUnit> &SUnits,
   }
   assert(Max && "Failed to find bottom of the critical path");
 
 #ifndef NDEBUG
   {
     LLVM_DEBUG(dbgs() << "Critical path has total latency "
                       << (Max->getDepth() + Max->Latency) << "\n");
     LLVM_DEBUG(dbgs() << "Available regs:");
-    for (unsigned Reg = 0; Reg < TRI->getNumRegs(); ++Reg) {
+    for (unsigned Reg = 1; Reg < TRI->getNumRegs(); ++Reg) {
       if (KillIndices[Reg] == ~0u)
         LLVM_DEBUG(dbgs() << " " << printReg(Reg, TRI));
     }
     LLVM_DEBUG(dbgs() << '\n');
   }
 #endif
 
   // Track progress along the critical path through the SUnit graph as we walk