[PATCH] D119475: [AMDGPU] Add scheduler pass to rematerialize trivial defs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:799
+  // occupancy. Keep the modified live-in set from second check.
+  if (!ImprovedAfterSinkingLiveThrus) {
+    // Keep a list of newly rematerialized instructions so that we can easily
----------------
vangthao wrote:
> rampitec wrote:
> > vangthao wrote:
> > > rampitec wrote:
> > > > I do not think you really need a whole block above dealing with live-through estimations. First it is not guaranteed to help the RP if you sink a live-through. Second it is essentially the same as this block actually performing rematerialization, just less precise. Keep just rematerialization part.
> > > If the live-through is increasing RP in this block then by rematerializing don't we decrease RP by the live-throughs that we rematerialized?
> > I do not think LICM will hoist anything like that. Then even if so the code below will handle it as well.
> The code below only checks for trivially rematerializable defs used in the high RP block. If the def is live-through and used in a low RP block then it will not be checked. This checks for that and sinks those defs to lower RP blocks so we can decrease live-through RP.
I see 2 situations where something sinkable can be live-through:

1)
```
Def
loop:
  ...
cbr loop
Use
```
There is absolutely no reason for any pass to hoist that Def high, most likely it will be sunk much earlier.

2)
```
Def
loop1:
  loop2:
    Use
  cbr loop2
cbr loop1
```
Assuming you have a high live-through pressure in loop1 sinking Def to Use into loop2 will unlikely help it as the highest pressure is likely in the loop2 anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119475/new/

https://reviews.llvm.org/D119475