AMDGPU/SI: Fix two missing NodeNum checks for SISched

[axeldavy]

Summary

SIScheduleDAGMI::moveLowLatencies was potentially calling isLowLatencyInstruction for SUs with NodeNum >= DAGSize, which is illegal.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99994

[arsenm]

Missing test?

[axeldavy]

It was quite hard and a long work to find a test for this issue.

A COPY operation doesn't usually have ExitSU as successor.
The following test (extracted from crashing app) triggers this behaviour on llvm 5 though:

---
name:            sisched-crash
tracksRegLiveness: true
registers:
  - { id: 0, class: vreg_512, preferred-register: '' }
body:             |
  bb.0:
  liveins:
    undef %0.sub0 = V_MOV_B32_e32 -1063317674, implicit %exec
    %0.sub1 = V_MOV_B32_e32 -1069424300, implicit %exec
    %0.sub2 = V_MOV_B32_e32 -1073212503, implicit %exec
    %0.sub3 = V_MOV_B32_e32 -1077848048, implicit %exec
    %0.sub4 = V_MOV_B32_e32 -1081645570, implicit %exec
    %0.sub5 = V_MOV_B32_e32 -1091633047, implicit %exec
    %0.sub6 = V_MOV_B32_e32 1039433088, implicit %exec
    %0.sub7 = V_MOV_B32_e32 1060467916, implicit %exec
    %0.sub8 = V_MOV_B32_e32 1067795870, implicit %exec
    %0.sub9 = COPY %0.sub8
...

I've no clue why this particular sequence of instructions (I tried smaller %0 register, writing fewer subs and it doesn't fail !).

The test doesn't crash though when tested under llvm svn.

I don't see any other way of triggering the 'ExitSU among the successors of a COPY op'.
Maybe the bug is fixed with svn (I have no more time to test with mesa linked to llvm svn), but I'd suggest merge the fix anyway to be more future-proof.

[arsenm]

Is this still an issue?

[axeldavy]

In D47984#1407111, @arsenm wrote:

Is this still an issue?

I don't know if the issue can still be triggered, but the fix won't harm.

[arsenm]

Test would be nice

[nhaehnle]

LGTM

[arsenm]

Please rebase if still relevant

lib/Target/AMDGPU/SIMachineScheduler.cpp

void SIScheduleDAGMI::moveLowLatencies() {
 
    for (unsigned i = 0, e = ScheduledSUnits.size(); i != e; ++i) {
     SUnit *SU = &SUnits[ScheduledSUnits[i]];
     bool IsLowLatencyUser = false;
     unsigned MinPos = 0;
 
     for (SDep& PredDep : SU->Preds) {
       SUnit *Pred = PredDep.getSUnit();
+      if (Pred->NodeNum >= DAGSize)
+        continue;
       if (SITII->isLowLatencyInstruction(*Pred->getInstr())) {
         IsLowLatencyUser = true;
       }
-      if (Pred->NodeNum >= DAGSize)
-        continue;
       unsigned PredPos = ScheduledSUnitsInv[Pred->NodeNum];
       if (PredPos >= MinPos)
         MinPos = PredPos + 1;
     }
 
     if (SITII->isLowLatencyInstruction(*SU->getInstr())) {
       unsigned BestPos = LastLowLatencyUser + 1;
       if ((int)BestPos <= LastLowLatencyPos)
for (unsigned i = 0, e = ScheduledSUnits.size(); i != e; ++i) {
     } else if (IsLowLatencyUser) {
       LastLowLatencyUser = i;
     // Moves COPY instructions on which depends
     // the low latency instructions too.
     } else if (SU->getInstr()->getOpcode() == AMDGPU::COPY) {
       bool CopyForLowLat = false;
       for (SDep& SuccDep : SU->Succs) {
         SUnit *Succ = SuccDep.getSUnit();
-        if (SITII->isLowLatencyInstruction(*Succ->getInstr())) {
+        if (Succ->NodeNum < DAGSize &&
+            SITII->isLowLatencyInstruction(*Succ->getInstr())) {
           CopyForLowLat = true;
         }
       }
       if (!CopyForLowLat)
         continue;
       if (MinPos < i) {
         for (unsigned u = i; u > MinPos; --u) {
           ++ScheduledSUnitsInv[ScheduledSUnits[u-1]];