[PATCH] D57056: [MC][X86] Correctly model additional operand latency caused by transfer delays from the integer to the floating point unit.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb created this revision.
andreadb added reviewers: RKSimon, spatel, craig.topper, courbet, ab, atrick.
Herald added subscribers: lebedev.ri, gbedwell.
Herald added a reviewer: lebedev.ri.

This patch adds a new ReadAdvance definition named `ReadInt2Fpu`.
`ReadInt2Fpu` allows x86 scheduling models to accurately describe delays caused by data transfers from the integer unit to the floating point unit.
`ReadInt2Fpu` currently defaults to a delay of zero cycles (i.e. no delay) for all x86 models excluding BtVer2. That means, this patch is only a functional change for the Jaguar cpu model only.

Tablegen definitions for instructions (V)PINSR* have been updated to account for the new `ReadInt2Fpu`. That read is mapped to the the GPR input operand.
On Jaguar, int-to-fpu transfers are modeled as a +6cy delay. Before this patch, that extra delay was added to the opcode latency. In practice, the insert opcode only executes for 1cy. Most of the actual latency is actually contributed by the so-called operand-latency. According to the AMD SOG for family 16h, (V)PINSR* latency is defined by expression `f+1`, where `f` is defined as a forwarding delay from the integer unit to the fpu.

When printing instruction latency from MCA (see InstructionInfoView.cpp) and LLC (only when flag -print-schedule is speified), we now need to account for any extra forwarding delays. We do this by checking if scheduling classes declare any negative ReadAdvance entries.  Quoting a code comment in TargetSchedule.td: "A negative advance effectively increases latency, which may be used for cross-domain stalls". When computing the instruction latency for the purpose of our scheduling tests, we now add any extra delay to the formula. This avoids regressing existing codegen and mca schedule tests. It comes with the cost of an extra (but very simple) hook in MCSchedModel.

As a side note: this patch would have been a bit simpler if we didn't have to support flag `-print-schedule`. Now that we have llvm-mca, we can just move all our latency/throughput test coverage to llvm-mca. If we do that, then we can get rid of flag `-print-schedule`, and all the latency/throughput reporting framework. It would also help to solve a long-standing layering violation (originally reported as PR37160).

Let me know if okay to commit.

Thanks,
Andrea


https://reviews.llvm.org/D57056

Files:
  include/llvm/MC/MCSchedule.h
  include/llvm/MC/MCSubtargetInfo.h
  include/llvm/MCA/Instruction.h
  lib/CodeGen/TargetSubtargetInfo.cpp
  lib/MC/MCSchedule.cpp
  lib/MCA/InstrBuilder.cpp
  lib/Target/X86/X86InstrMMX.td
  lib/Target/X86/X86InstrSSE.td
  lib/Target/X86/X86SchedBroadwell.td
  lib/Target/X86/X86SchedHaswell.td
  lib/Target/X86/X86SchedSandyBridge.td
  lib/Target/X86/X86SchedSkylakeClient.td
  lib/Target/X86/X86SchedSkylakeServer.td
  lib/Target/X86/X86Schedule.td
  lib/Target/X86/X86ScheduleAtom.td
  lib/Target/X86/X86ScheduleBdVer2.td
  lib/Target/X86/X86ScheduleBtVer2.td
  lib/Target/X86/X86ScheduleSLM.td
  lib/Target/X86/X86ScheduleZnver1.td
  test/CodeGen/X86/mmx-schedule.ll
  test/CodeGen/X86/sse41-schedule.ll
  test/tools/llvm-mca/X86/BtVer2/int-to-fpu-forwarding-1.s
  test/tools/llvm-mca/X86/BtVer2/int-to-fpu-forwarding-3.s
  tools/llvm-mca/Views/InstructionInfoView.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57056.182909.patch
Type: text/x-patch
Size: 21921 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190122/751ba735/attachment.bin>