[llvm-branch-commits] [llvm] [2/3][AMDGPU] Physical register tracking in GCN trackers. (PR #184275)

[Dhruva Chakrabarti via llvm-branch-commits]

================
@@ -320,12 +325,72 @@ class GCNRPTracker {
 
 protected:
   const LiveIntervals &LIS;
+
   LiveRegSet VirtLiveRegs;
+
+  // Physical register liveness: Units provides O(1) unit-level alias checks,
+  // Regs tracks which register names contributed to pressure for cheap
+  // reconstruction. Both must be kept in sync.
----------------
dhruvachak wrote:

Good point. Yes, when the UpwardTracker uses recede(), it can indeed lead to the above described scenario, leading to under-counting of register pressure. In order to be accurate, we would need a more expensive operation for every def. Specifically, we would need to check which entries in Regs have their units fully covered by the def, remove those entries from Regs, and decrement register pressure for each of them. Since this kind of overlap should be rare in practice, to keep things efficient, I am leaning towards not making this change. Instead, I will document this potential issue.

There are a couple of other cases where under/over-counting of register pressure because of physical registers may occur. The current implementation of physical register tracking does not consider live-ins/live-outs, so that may lead to accuracy issues. Computing live-ins/live-outs may not be cheap, so I wanted to leave them out of the first version. I will document that as well.

While thinking more about your example, I found a potential inconsistency because of which validation of register pressure may fail. The method recede() validates the total pressure and then there is the isValid() method on the tracker. I will push up a fix to that problem.

https://github.com/llvm/llvm-project/pull/184275