[MachineScheduler] Clear subregister entries also in addPhysRegDeps when handling a def

[jonpa]

Summary

When working with the SystemZ InstRWs, I found it disturbing to find edges that were seemingly unnecessary in the DAGs.

To my surprise I found that when going bottom-up while building the schedgraph, the uses/defs entries for subregs of Reg were not removed. I was expecting this since those earlier subregs cannot use a later seen (bottom-up) def of that subreg, due to the current def of Reg. Is there a reason that this is not done?

My patch is very nearly NFC on SystemZ benchmarks. In fact, just 2 or 4 instructions on SPEC move around, so it's not strictly NFC, but... I think that case was due to some height difference that resulted from such an edge for a subreg around the Reg SU. Of course, if SU(0) defines Reg:sub, SU(1) defines Reg, and SU(2) uses Reg:Sub, there is no use to have an edge from SU(0) to SU(2) for Reg:sub, since Reg already forces edges from SU(0) -> SU(1) and SU(1) -> SU(2).

It seems like a good idea to minimize the number of DAG edges if there is no particular reason to have them, for performance reasons. It also makes the DAG easier to read and handle.

It could be that I am missing something here, for instance is a subreg always completely covered by Reg? In that case this patch only makes sense on targets that don't have that property, unfortunately.

[atrick]

We didn't support cross-call scheduling last I looked at this. Hopefully @MatzeB can verify that your fix is correct.

[jonpa]

In D46838#1098248, @atrick wrote:

We didn't support cross-call scheduling last I looked at this. Hopefully @MatzeB can verify that your fix is correct.

I did not intend to change anything in regards to the handling of calls, just to also handle subregs with Uses and Defs. Did I miss something?

Is there some other user of ScheduleDAGInstrs::buildSchedGraph() than MachineScheduler that allows rescheduling around calls, and that's why the handling of calls is here?

[atrick]

LGTM, ignore my previous comment and sorry for the delay.

[jonpa]

Committed as r333165.

lib/CodeGen/ScheduleDAGInstrs.cpp

if (!MO.isDef()) {
     // retrieve the existing SUnits list for this register's uses.
     // Push this SUnit on the use list.
     Uses.insert(PhysRegSUOper(SU, OperIdx, Reg));
     if (RemoveKillFlags)
       MO.setIsKill(false);
   } else {
     addPhysRegDataDeps(SU, OperIdx);
 
-    // clear this register's use list
-    if (Uses.contains(Reg))
-      Uses.eraseAll(Reg);
-
-    if (!MO.isDead()) {
-      Defs.eraseAll(Reg);
-    } else if (SU->isCall) {
+    // Clear previous uses and defs of this register and its subergisters.
+    for (MCSubRegIterator SubReg(Reg, TRI, true); SubReg.isValid(); ++SubReg) {
+      if (Uses.contains(*SubReg))
+        Uses.eraseAll(*SubReg);
+      if (!MO.isDead())
+        Defs.eraseAll(*SubReg);
+    }
+    if (MO.isDead() && SU->isCall) {
       // Calls will not be reordered because of chain dependencies (see
       // below). Since call operands are dead, calls may continue to be added
       // to the DefList making dependence checking quadratic in the size of
       // the block. Instead, we leave only one call at the back of the
       // DefList.
       Reg2SUnitsMap::RangePair P = Defs.equal_range(Reg);
       Reg2SUnitsMap::iterator B = P.first;
       Reg2SUnitsMap::iterator I = P.second;