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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D38164#879078, @MatzeB wrote:

> - tryPressure seems like the wrong place to me, as it is used in 3 different contexts: (compared with target limits, compared with increase region limits, and the current max). From your description it sounds like we only want this behavior once.


I tried only favoring instructions that do not increase pressure in each context.  When using the current max, the results are very similar to the initial version of the patch and the AMDGPU failures go away. Using it in the other context yields smaller improvements on the set of benchmarks mentioned above.

In https://reviews.llvm.org/D38164#880843, @MatzeB wrote:

> I think the cryptocode I was looking at back then was john the ripper (openbenchmarking.org / pts/john-the-ripper).
>
> Just tried with/without your changes and I see the descrypt part improving and the LM part regressing. So whether this is an improvement or regression may depend on the situation and we should find/look at examples.


I'll try to collect a list of examples where it makes things better/worse.

> I assume Andy designed the current behavior to give more freedom to the latency minimizing code? So an out of order Cortex-A57 may not be the best test case for it; then again if it improves the spilling situation and leads to a win on out of order cores we could switch the behavior based on out-of-order/in-order.

I'll also run the benchmarks on an in-order core. I was under the impression that the scheduler prioritizes register needs over latency, as the latency of candidates is one of the last things checked in `tryCandidate`.



================
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;
+  }
----------------
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.


https://reviews.llvm.org/D38164