[PATCH] D38279: [MachineScheduler] Enable latency heuristic based on scheduled lat.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D38279#937309, @jonpa wrote:

> This makes sense to me as well, but I wonder why Issued gets a different value depending on top/bot ?


My understanding was current cycle is quite accurate from top-down, but not for bottom up. For example, after scheduling a node with latency 3 from bottom, we would only bump CurrCycle to 1. But if this node is on the critical path, scheduling the predecessor in the next cycle increases the critical path by 2 cycles (if we there are enough remaining instructions to be scheduled), because it was scheduled too early.

When going top-down, only after CurrCycle + RemLatency > CriticalPath scheduling a node later would increase the critical path.

Another way to think about it: When going top down, scheduling a node on the critical path “too early” does not have any impact on that critical path (it might cause another path to become critical though). But scheduling a node on the critical path “too early” when going bottom up is worse, as each instruction scheduled after the node increases the critical path ( assuming we can issue 1 instruction per cycle).



================
Comment at: lib/CodeGen/MachineScheduler.cpp:2445
+    unsigned Issued = CurrZone.isTop() ? CurrZone.getCurrCycle() :
+                                         CurrZone.getScheduledLatency();
+    if (IsPostRA || (RemLatency + Issued > Rem.CriticalPath)) {
----------------
javed.absar wrote:
> Bit strange. getScheduledLatency returns max of CurrCycle and ExpectedLatency which I dont quite find set anywhere (other than 0). 
It's updated in `bumpNode`, through the reference created at line 2230 I think.


https://reviews.llvm.org/D38279