[PATCH] D30941: Better testing of schedule model instruction latencies/throughputs

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1278
+  int Latency = 0;
+  for (unsigned OpIdx = 0, OpIdxEnd = Inst->getNumOperands(); OpIdx != OpIdxEnd;
+       ++OpIdx)
----------------
Can you call TII->getInstrLatency here instead of computing it in this loop?

(if you just call SCModel->computeInstrLatency, as suggested below, this will take care of itself).


================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1291
+  auto &STI = Inst->getParent()->getParent()->getSubtarget();
+  auto &SCModel = STI.getSchedModel();
+  const int NoInformationAvailable = -1;
----------------
Can't you just call SCModel->computeInstrLatency return the result? The logic there seems like exactly what you want:

  unsigned
  TargetSchedModel::computeInstrLatency(const MachineInstr *MI,
                                        bool UseDefaultDefLatency) const {
    // For the itinerary model, fall back to the old subtarget hook.
    // Allow subtargets to compute Bundle latencies outside the machine model.
    if (hasInstrItineraries() || MI->isBundle() ||
        (!hasInstrSchedModel() && !UseDefaultDefLatency))
      return TII->getInstrLatency(&InstrItins, *MI);
 
    if (hasInstrSchedModel()) {
      const MCSchedClassDesc *SCDesc = resolveSchedClass(MI);
      if (SCDesc->isValid())
        return computeInstrLatency(*SCDesc);
    }
    return TII->defaultDefLatency(SchedModel, *MI);
  }
 



================
Comment at: lib/Target/X86/X86MCInstLower.cpp:1333
 
+  if (TM.Options.MCOptions.PrintLatency ||
+      MI->getAsmPrinterFlags() & PRINT_LATENCY) {
----------------
This seems really useful, but is not target dependent. Can you please move this hook into the target-independent code? Maybe in void AsmPrinter::EmitFunctionBody(), around here:

      default:
        EmitInstruction(&MI);
        break;

(right before the call to EmitInstruction).



https://reviews.llvm.org/D30941