[PATCH] D33087: [AMDGCN] Fix overly optimistic GCNUpwardRPTracker

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec created this revision.
Herald added subscribers: nhaehnle, arsenm.

1. Remove defs when moving past the defining instruction, not right on it. If an instruction defines and uses a register its pressure is actually 2, not 0.
2. Do not remove tied defs. If def is a tied operand it is not really a new def, just a redefinition.


Repository:
  rL LLVM

https://reviews.llvm.org/D33087

Files:
  lib/Target/AMDGPU/GCNIterativeScheduler.cpp
  lib/Target/AMDGPU/GCNRegPressure.cpp


Index: lib/Target/AMDGPU/GCNRegPressure.cpp
===================================================================
--- lib/Target/AMDGPU/GCNRegPressure.cpp
+++ lib/Target/AMDGPU/GCNRegPressure.cpp
@@ -262,29 +262,31 @@
 void GCNUpwardRPTracker::recede(const MachineInstr &MI) {
   assert(MRI && "call reset first");
 
+  // process all defs first to ensure early clobbers are handled correctly
+  // iterating over operands() to catch implicit defs
+  if (LastTrackedMI && !LastTrackedMI->isDebugValue()) {
+    for (const auto &MO : LastTrackedMI->operands()) {
+      if (!MO.isReg() || !MO.isDef() || MO.isTied() ||
+          !TargetRegisterInfo::isVirtualRegister(MO.getReg()))
+        continue;
+
+      auto Reg = MO.getReg();
+      auto &LiveMask = LiveRegs[Reg];
+      auto PrevMask = LiveMask;
+      LiveMask &= ~getDefRegMask(MO);
+      CurPressure.inc(Reg, PrevMask, LiveMask, *MRI);
+    }
+  }
+
   LastTrackedMI = &MI;
 
   if (MI.isDebugValue())
     return;
 
-  // process all defs first to ensure early clobbers are handled correctly
-  // iterating over operands() to catch implicit defs
-  for (const auto &MO : MI.operands()) {
-    if (!MO.isReg() || !MO.isDef() ||
-      !TargetRegisterInfo::isVirtualRegister(MO.getReg()))
-      continue;
-
-    auto Reg = MO.getReg();
-    auto &LiveMask = LiveRegs[Reg];
-    auto PrevMask = LiveMask;
-    LiveMask &= ~getDefRegMask(MO);
-    CurPressure.inc(Reg, PrevMask, LiveMask, *MRI);
-  }
-
   // then all uses
   for (const auto &MO : MI.uses()) {
     if (!MO.isReg() || !MO.readsReg() ||
-      !TargetRegisterInfo::isVirtualRegister(MO.getReg()))
+        !TargetRegisterInfo::isVirtualRegister(MO.getReg()))
       continue;
 
     auto Reg = MO.getReg();
Index: lib/Target/AMDGPU/GCNIterativeScheduler.cpp
===================================================================
--- lib/Target/AMDGPU/GCNIterativeScheduler.cpp
+++ lib/Target/AMDGPU/GCNIterativeScheduler.cpp
@@ -236,6 +236,9 @@
     UPTracker.recede(*I);
 
   UPTracker.recede(*Begin);
+  // Second recede to the same instruction to commit all defs.
+  // Current pressure will be updated, but max will stay higher.
+  UPTracker.recede(*Begin);
 
   assert(UPTracker.isValid() ||
          (dbgs() << "Tracked region ",


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33087.98575.patch
Type: text/x-patch
Size: 2261 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170511/36a730a4/attachment.bin>