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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa created this revision.
jonpa added reviewers: atrick, MatzeB.

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.


https://reviews.llvm.org/D46838

Files:
  lib/CodeGen/ScheduleDAGInstrs.cpp


Index: lib/CodeGen/ScheduleDAGInstrs.cpp
===================================================================
--- lib/CodeGen/ScheduleDAGInstrs.cpp
+++ lib/CodeGen/ScheduleDAGInstrs.cpp
@@ -318,13 +318,14 @@
   } 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46838.146639.patch
Type: text/x-patch
Size: 995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180514/ca074519/attachment.bin>