[mips] Fix forbidden slot hazard handling

[sdardis]

Summary

MipsHazardSchedule has to determine what the next physical machine instruction
is to decide whether to insert a nop. In case where a branch with a forbidden
slot appears at the end of a basic block, first *real* instruction of the next
physical basic block was determined using getFirstNonDebugInstr().

Unfortunately this only considers DBG_VALUEs and not other transient opcodes
such as EHLABEL. As EHLABEL passes the SafeInForbiddenSlot predicate and the
instruction after the EHLABEL can be a CTI, we observed test failures in the
LNT testsuite.

[sdardis]

Ping. I have not been able to add a test case to cover this, as the MIR parser doesn't yet cover parsing EH_LABELs. The LLVM test suite covers the change:

MultiSource/Benchmarks/Bullet/bullet
MultiSource/Benchmarks/PAQ8p/paq8p
SingleSource/Benchmarks/Misc-C++-EH/spirit
SingleSource/Benchmarks/Misc-C++/bigfib
SingleSource/Benchmarks/Shootout-C++/lists1

Those tests fail without this change, as EH_LABEL is non-debug instruction causing a sequence of a compact branch followed by a jal.

[sdardis]

Ping

[dsanders]

LGTM with a couple nits

lib/Target/Mips/MipsHazardSchedule.cpp

98–99

This loop seems equivalent to:

return std::find_if(I, E, [](const auto &Insn) { return !Insn->isTransient(); })

Also, we should assert that we don't return E.

[sdardis]

Committed as rL268052.

lib/Target/Mips/MipsHazardSchedule.cpp

 char MipsHazardSchedule::ID = 0;
 } // end of anonymous namespace
 
 /// Returns a pass that clears pipeline hazards.
 FunctionPass *llvm::createMipsHazardSchedule() {
   return new MipsHazardSchedule();
 }
 
+// Find the next real instruction from the current position.
+static Iter getNextMachineInstr(Iter Position) {
+  Iter I = Position, E = Position->getParent()->end();
+  while (I != E && I->isTransient())
+    I++;
+
+  return I;
+}
+
 bool MipsHazardSchedule::runOnMachineFunction(MachineFunction &MF) {
 
   const MipsSubtarget *STI =
       &static_cast<const MipsSubtarget &>(MF.getSubtarget());
 
   // Forbidden slot hazards are only defined for MIPSR6.
   if (!STI->hasMips32r6() || STI->inMicroMipsMode())
     return false;
 
   bool Changed = false;
   const MipsInstrInfo *TII = STI->getInstrInfo();
 
   for (MachineFunction::iterator FI = MF.begin(); FI != MF.end(); ++FI) {
     for (Iter I = FI->begin(); I != FI->end(); ++I) {
 
       // Forbidden slot hazard handling. Use lookahead over state.
       if (!TII->HasForbiddenSlot(*I))
         continue;
 
       bool InsertNop = false;
       // Next instruction in the basic block.
       if (std::next(I) != FI->end() &&
-          !TII->SafeInForbiddenSlot(*std::next(I))) {
+          !TII->SafeInForbiddenSlot(*getNextMachineInstr(std::next(I)))) {
         InsertNop = true;
       } else {
         // Next instruction in the physical successor basic block.
         for (auto *Succ : FI->successors()) {
           if (FI->isLayoutSuccessor(Succ) &&
-              Succ->getFirstNonDebugInstr() != Succ->end() &&
-              !TII->SafeInForbiddenSlot(*Succ->getFirstNonDebugInstr())) {
+              getNextMachineInstr(Succ->begin()) != Succ->end() &&
+              !TII->SafeInForbiddenSlot(*getNextMachineInstr(Succ->begin()))) {
             InsertNop = true;
             break;
           }
         }
       }
 
       if (InsertNop) {
         Changed = true;
         MIBundleBuilder(I)
             .append(BuildMI(MF, I->getDebugLoc(), TII->get(Mips::NOP)));
         NumInsertedNops++;
       }
     }
   }
   return Changed;
 }