[PATCH] D38164: [MachineScheduler] Favor instructions that do not increase pressure.

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 11 16:47:51 PDT 2017


MatzeB requested changes to this revision.
MatzeB added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/CodeGen/MachineScheduler.cpp:2777-2788
+
+  // If one of the candidates does neither increase or decrease pressure, but
+  // the other does increase the pressure, go with it.
+  if (TryP.getUnitInc() == 0 && CandP.getUnitInc() > 0) {
+    TryCand.Reason = Reason;
+    return true;
+  }
----------------
fhahn wrote:
> MatzeB wrote:
> > Why isn't this using `tryGreate()` and isn't this the same as just changing `getUnitInc() < 0` to `getUnitInc() <= 0` in the earlier check?
> Initially I used `tryGreater` above with `getUnitInc() <= 0`. But that version does not prioritize `TryCand` over `Cand` if `TryCand` decreases pressure and `Cand` does not change pressure I think.
I just looked at this patch again:
There actually already is `tryLess(TryP.getUnitInc(), CandP.getUnitInc(), TryCand, Cand, Reason);` below. And the important part here is that it is guarded by `Cand.AtTop != TryCand.AtTop`. Last time I meddled with comparing candidates across the top/bottom boundary I found that we tend to produce terrible schedules for long basic blocks (well Tom complained because of AMDGPU regressions and as it turns out GPUs are more likely to have big scheduling regions). The reason is that it is easy to make decisions on one boundary that run contrary to the decisions being made at the other boundary. With that in mind I'd rather reject this patch or to convince me I would like to some concrete situations or test cases.


https://reviews.llvm.org/D38164





More information about the llvm-commits mailing list