[MCA] [TimelineView] Fixing a bug (that I caused) in TimelineView.cpp that was causing instructions that execute after timeline-max-cycles to still be displayed.

[holland11]

Summary

Within this diff https://reviews.llvm.org/D104675, I tried to fix a bug that was causing 0 latency instructions that were being retired within cycle 0 to break the timeline view. The way I fixed it was insufficient, because I didn't understand the reason for the if statement in TimelineView::printTimeline(). I now understand its purpose so I've modified my 'fix' to still behave as intended, but to also still fix the 0 latency issue.

To be more clear about what's happening, let's look at TimelineView::onEvent():

void TimelineView::onEvent(const HWInstructionEvent &Event) {
  const unsigned Index = Event.IR.getSourceIndex();
  if (Index >= Timeline.size())
    return;

  switch (Event.Type) {
  case HWInstructionEvent::Retired: {
    TimelineViewEntry &TVEntry = Timeline[Index];
    if (CurrentCycle < MaxCycle)
      TVEntry.CycleRetired = CurrentCycle;

    // Update the WaitTime entry which corresponds to this Index.
    assert(TVEntry.CycleDispatched >= 0 && "Invalid TVEntry found!");
    unsigned CycleDispatched = static_cast<unsigned>(TVEntry.CycleDispatched);
    WaitTimeEntry &WTEntry = WaitTime[Index % getSource().size()];
    WTEntry.CyclesSpentInSchedulerQueue +=
        TVEntry.CycleIssued - CycleDispatched;
    assert(CycleDispatched <= TVEntry.CycleReady &&
           "Instruction cannot be ready if it hasn't been dispatched yet!");
    WTEntry.CyclesSpentInSQWhileReady +=
        TVEntry.CycleIssued - TVEntry.CycleReady;
    if (CurrentCycle > TVEntry.CycleExecuted) {
      WTEntry.CyclesSpentAfterWBAndBeforeRetire +=
          (CurrentCycle - 1) - TVEntry.CycleExecuted;
    }
    break;
  }
  case HWInstructionEvent::Ready:
    Timeline[Index].CycleReady = CurrentCycle;
    break;
  case HWInstructionEvent::Issued:
    Timeline[Index].CycleIssued = CurrentCycle;
    break;
  case HWInstructionEvent::Executed:
    Timeline[Index].CycleExecuted = CurrentCycle;
    break;
  case HWInstructionEvent::Dispatched:
    // There may be multiple dispatch events. Microcoded instructions that are
    // expanded into multiple uOps may require multiple dispatch cycles. Here,
    // we want to capture the first dispatch cycle.
    if (Timeline[Index].CycleDispatched == -1)
      Timeline[Index].CycleDispatched = static_cast<int>(CurrentCycle);
    break;
  default:
    return;
  }
  if (CurrentCycle < MaxCycle)
    LastCycle = std::max(LastCycle, CurrentCycle);
}

We can see that when an instruction is Retired, if CurrentCycle is not less than MaxCycle, CycleRetired stays 0. However, when an instruction is Executed, we set CycleExecuted regardless.

So the new change that I am making now will check for CycleRetired == 0 to determine whether this instruction was Retired within the timeline limits, but it will also check CycleExecuted to make sure we aren't looking at a 0 cycle instruction that has a legitimate CycleRetired set to 0.

It's possible that during these few days where the bug was present, some new llvm-mca related tests were made (that could now fail). But I ran both ninja check all and ninja check llvm-mca and there were no failures so we should be fine.

[andreadb]

LGTM

llvm/tools/llvm-mca/Views/TimelineView.cpp

void TimelineView::printTimeline(raw_ostream &OS) const {
   FOS.flush();
 
   unsigned IID = 0;
   ArrayRef<llvm::MCInst> Source = getSource();
   const unsigned Iterations = Timeline.size() / Source.size();
   for (unsigned Iteration = 0; Iteration < Iterations; ++Iteration) {
     for (const MCInst &Inst : Source) {
       const TimelineViewEntry &Entry = Timeline[IID];
+      // When an instruction is retired after timeline-max-cycles,
+      // its CycleRetired is left at 0. However, it's possible for
+      // a 0 latency instruction to be retired during cycle 0 and we
+      // don't want to early exit in that case. The CycleExecuted
+      // attribute is set correctly whether or not it is greater
+      // than timeline-max-cycles so we can use that to ensure
+      // we don't early exit because of a 0 latency instruction.
+      if (Entry.CycleRetired == 0 && Entry.CycleExecuted != 0)
+        return;
 
       unsigned SourceIndex = IID % Source.size();
       printTimelineViewEntry(FOS, Entry, Iteration, SourceIndex);
       FOS << "   " << printInstructionString(Inst) << '\n';
       FOS.flush();
 
       ++IID;
     }