This is an archive of the discontinued LLVM Phabricator instance.

[MISched] Use StartAtCycle in trace dumps.
ClosedPublic

Authored by fpetrogalli on May 10 2023, 2:29 PM.

Details

Summary

This commit re-work the methods that dump traces with resource usage to take into account the StartAtCycle value added by https://reviews.llvm.org/D150310.

For each i, the values of the lists StartAtCycle and ReservedCycles is are printed with the interval [StartAtCycle[i], ReservedCycles[i])

... | StartAtCycle[i] | ... | ReservedCycles[i] - 1 | ReservedCycles[i] | ...
    | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |                   |

Diff Detail

Event Timeline

fpetrogalli created this revision.May 10 2023, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 2:29 PM
fpetrogalli requested review of this revision.May 10 2023, 2:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 2:29 PM
andreadb added inline comments.May 12 2023, 4:24 AM
llvm/lib/CodeGen/MachineScheduler.cpp
164

s/respource/resource

1004–1009

What if you use a SmallVector here?
I may remember it wrongly, but I think that there is a constructor which would allow you to pass an iterator range.

1024

Can this assert ever trigger in practice? I thought it was already tested by your logic from D150310.

RKSimon added inline comments.May 12 2023, 7:31 AM
llvm/lib/CodeGen/MachineScheduler.cpp
165

Add test coverage for this switch?

1025

(style)

for (unsigned I = 0, E = PI->Cycles - PI->StartAtCycle; I != E; ++i, ++C)
1095

use llvm::sort

1105

(style)

for (unsigned I = 0, E = PI->Cycles - PI->StartAtCycle; I != E; ++I, --C)

Also - please add a summary to the patch

fpetrogalli edited the summary of this revision. (Show Details)May 15 2023, 6:15 AM
fpetrogalli marked 7 inline comments as done.
fpetrogalli retitled this revision from [MISched] Use StartAtCycle in trace dumps. to [MISched] Use StartAtCycle in trace dumps..

Address code review.

andreadb accepted this revision.Jun 5 2023, 7:51 AM

LGTM

This revision is now accepted and ready to land.Jun 5 2023, 7:51 AM

Added early exit from the methods if there is no scheduling model attached to the CPU.

This revision was automatically updated to reflect the committed changes.

@fpetrogalli We're seeing build failures on EXPENSIVE_CHECKS builds due to the llvm::sort calls: https://lab.llvm.org/buildbot/#/builders/16/builds/49592 - you might be better off using llvm::stable_sort

@fpetrogalli We're seeing build failures on EXPENSIVE_CHECKS builds due to the llvm::sort calls: https://lab.llvm.org/buildbot/#/builders/16/builds/49592 - you might be better off using llvm::stable_sort

Thank you for the heads up. This should fix it D152800.