[PATCH] D141289: [MIScheduler] Print top/down cycle in the SUnit dump.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/CodeGen/ScheduleDAG.cpp:366
+    if (SchedPrintCycles)
+      dbgs() << " " << SU.TopReadyCycle << " " << SU.BotReadyCycle;
+    dbgs()<< ")";
----------------
Hmm, is there a reason for including it in the name and not including them among the attributes (`SUnit::dumpAttributes`)? There it would also be possible to print a name associated with the values.


================
Comment at: llvm/test/CodeGen/AArch64/sched-print-cycle.mir:18
+# CHECK-LABEL: *** Final schedule for %bb.0 ***
+# CHECK: SU(0 {{[0-9]+}} {{[0-9]+}}):   $x1 = ADDXrr $x0, $x0
+# CHECK: SU(1 {{[0-9]+}} {{[0-9]+}}):   $x2 = ADDXrr $x1, $x1
----------------
fpetrogalli wrote:
> fpetrogalli wrote:
> > fhahn wrote:
> > > Should this not check for the expected cycles?
> > I could, but what I am really interested in checking is the fact that we get 2 extra numbers with the option, not their values (for now). The scheduler tests will take care of checking the actual values. Is it OK if I live it as it is?
> To be more precise, I do not want to check for specific value because I do not want to bother updating the test in case they change. All I care is that the two extra numbers are there when `-sched-print-cycles=true`
If you fix the CPU (`-mcpu=xxx`) to an older one, it's unlikely that the latencies for ADD will change? It seems better to check that the correct values are printed IMO , rather than matching any value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141289