This is an archive of the discontinued LLVM Phabricator instance.

Differential D35749

RA: Remove another assert on empty intervals
ClosedPublic

Authored by arsenm on Jul 21 2017, 5:12 PM.

Details

Reviewers
MatzeB
qcolombet
Summary

This case is similar to the one fixed in r308808,
except when rematerializing.




Fixes bug 33884.
Diff Detail
Event Timeline
arsenm created this revision.Jul 21 2017, 5:12 PM
Herald added subscribers: eraman, nhaehnle, wdng.  ·  View Herald TranscriptJul 21 2017, 5:12 PM
MatzeB accepted this revision.Jul 21 2017, 5:16 PM
LGTM
This revision is now accepted and ready to land.Jul 21 2017, 5:16 PM
arsenm closed this revision.Jul 21 2017, 5:26 PM
r308813
qcolombet edited edge metadata.Jul 21 2017, 5:53 PM
Before removing all those assertions, could we talk that through first?

We started a conversion in D35730.
Revision Contents
PathSize
lib/
CodeGen/
4 lines
test/
CodeGen/
AMDGPU/
34 lines
Diff 107758
lib/CodeGen/InlineSpiller.cpp
Show First 20 Lines • Show All 637 Lines • ▼ Show 20 Linesvoid InlineSpiller::reMaterializeAll() {

  // So to get rid of unused reg, we need to check whether it has non-dbg
  // So to get rid of unused reg, we need to check whether it has non-dbg

  // reference instead of whether it has non-empty interval.
  // reference instead of whether it has non-empty interval.

  unsigned ResultPos = 0;
  unsigned ResultPos = 0;

  for (unsigned Reg : RegsToSpill) {
  for (unsigned Reg : RegsToSpill) {

    if (MRI.reg_nodbg_empty(Reg)) {
    if (MRI.reg_nodbg_empty(Reg)) {

      Edit->eraseVirtReg(Reg);
      Edit->eraseVirtReg(Reg);

      continue;
      continue;

    }
    }

    assert((LIS.hasInterval(Reg) && !LIS.getInterval(Reg).empty()) &&


           "Reg with empty interval has reference");
    assert(LIS.hasInterval(Reg) && "Reg with no interval");

    RegsToSpill[ResultPos++] = Reg;
    RegsToSpill[ResultPos++] = Reg;

  }
  }

  RegsToSpill.erase(RegsToSpill.begin() + ResultPos, RegsToSpill.end());
  RegsToSpill.erase(RegsToSpill.begin() + ResultPos, RegsToSpill.end());

  DEBUG(dbgs() << RegsToSpill.size() << " registers to spill after remat.\n");
  DEBUG(dbgs() << RegsToSpill.size() << " registers to spill after remat.\n");

}
}







//===----------------------------------------------------------------------===//
//===----------------------------------------------------------------------===//

▲ Show 20 Lines • Show All 828 Lines • Show Last 20 Lines
test/CodeGen/AMDGPU/spill-empty-live-interval.mir
Show All 32 Linesbody:             |




  bb.1:
  bb.1:

    S_NOP 0, implicit %3.sub1
    S_NOP 0, implicit %3.sub1

    S_NOP 0, implicit %0.sub1
    S_NOP 0, implicit %0.sub1

    S_NOP 0, implicit undef %0.sub0
    S_NOP 0, implicit undef %0.sub0

    S_ENDPGM
    S_ENDPGM




...
...



# Similar assert which happens when trying to rematerialize.

# https://bugs.llvm.org/show_bug.cgi?id=33884

---

# CHECK-LABEL: name: rematerialize_empty_interval_has_reference



# CHECK-NOT: MOV

# CHECK: undef %3.sub2 = V_MOV_B32_e32 1786773504, implicit %exec



# CHECK: bb.1:

# CHECK-NEXT: S_NOP 0, implicit %3.sub2

# CHECK-NEXT: S_NOP 0, implicit undef %6.sub0

# CHECK-NEXT: undef %4.sub2 = V_MOV_B32_e32 0, implicit %exec

# CHECK-NEXT: S_NOP 0, implicit %4.sub2

name: rematerialize_empty_interval_has_reference

tracksRegLiveness: true

registers:

  - { id: 0, class: vreg_128, preferred-register: '' }

  - { id: 1, class: vgpr_32, preferred-register: '' }

  - { id: 2, class: vgpr_32, preferred-register: '' }

  - { id: 3, class: vreg_128, preferred-register: '' }

body:             |

  bb.0:

    successors: %bb.1



    undef %0.sub2 = V_MOV_B32_e32 0, implicit %exec

    undef %3.sub2 = V_MOV_B32_e32 1786773504, implicit %exec



  bb.1:

    S_NOP 0, implicit %3.sub2

    S_NOP 0, implicit undef %0.sub0

    S_NOP 0, implicit %0.sub2



...