================
@@ -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
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
