[PATCH] D23551: [mips] interAptiv based generic schedule model

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with the ResourceCycles corrected, a couple other minor changes, and an explanation for the dsr-fixed-objects.ll change


================
Comment at: lib/Target/Mips/Mips.td:194
@@ -193,3 +194,3 @@
 class Proc<string Name, list<SubtargetFeature> Features>
- : Processor<Name, MipsGenericItineraries, Features>;
 
----------------
If MipsGenericItineraries is no longer used can we delete it (in a follow-up patch)?

================
Comment at: lib/Target/Mips/MipsScheduleGeneric.td:202-203
@@ +201,4 @@
+// ======
+//def : ItinRW<[GenericReadCOP0], [II_MFHC0]>;
+//def : ItinRW<[GenericWriteCOP0], [II_MTHC0]>;
+
----------------
Could you uncomment these and add II_MFHC0 to the microMIPSR6 definitions? We don't seem to have MFHC0 for the other ISA's it exists for.

================
Comment at: lib/Target/Mips/MipsScheduleGeneric.td:260
@@ +259,3 @@
+def : ItinRW<[GenericWriteLoad], [II_LBE, II_LBUE, II_LHE, II_LHUE, II_LWE,
+                                   II_LLE]>;
+
----------------
Indentation

================
Comment at: lib/Target/Mips/MipsScheduleGeneric.td:349-380
@@ +348,34 @@
+def GenericWriteFPUMoveGPRFPU : SchedWriteRes<[GenericIssueFPUMove]> { let Latency = 2; }
+def GenericWriteFPUDivS : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 17;
+  let ResourceCycles = [ 1, 14 ];
+}
+def GenericWriteFPUDivD : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 32;
+  let ResourceCycles = [ 1, 29 ];
+}
+def GenericWriteFPURcpS : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 13;
+  let ResourceCycles = [ 1, 10 ];
+}
+def GenericWriteFPURcpD : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 25;
+  let ResourceCycles = [ 1, 21 ];
+}
+def GenericWriteFPURsqrtS : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 17;
+  let ResourceCycles = [ 1, 14 ];
+}
+def GenericWriteFPURsqrtD : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 32;
+  let ResourceCycles = [ 1, 29 ];
+}
+def GenericWriteFPUSqrtS : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 17;
+  let ResourceCycles = [ 1, 14 ];
+}
+def GenericWriteFPUSqrtD : SchedWriteRes<[GenericFPUDivSqrt]> {
+  let Latency = 29;
+  let ResourceCycles = [ 1, 29 ];
+}
+
----------------
I doubt these are working as intended since there are two ResourceCycles elements but only one resource and ResourceCycles are paired up with the resources. I think the current definition for GenericWriteFPUSqrtD is saying that it consumes the GenericFPUDivSqrt resource for a single cycle which is equivalent to a fully-pipelined divide path.

I think you need to remove each of the '1' elements since these corresponded with an issue port on the P5600 definitions.

Likewise for GenericDiv other places where ResourceCycles is set.

================
Comment at: lib/Target/Mips/MipsScheduleGeneric.td:436
@@ +435,3 @@
+def : ItinRW<[GenericWriteFPUMoveGPRFPU], [II_CFC1, II_CTC1, II_MFC1, II_MFHC1,
+                                        II_MTC1, II_MTHC1]>;
+
----------------
Indentation

================
Comment at: lib/Target/Mips/MipsScheduleGeneric.td:440
@@ +439,3 @@
+def : ItinRW<[GenericWriteFPUStore], [II_SDC1, II_SDXC1, II_SUXC1, II_SWC1,
+                                       II_SWXC1]>;
+
----------------
Indentation

================
Comment at: test/DebugInfo/Mips/dsr-fixed-objects.ll:32
@@ -31,3 +31,3 @@
 ; F0: [[LOC]]: Beginning address offset: 0x0000000000000028
-; F0:             Ending address offset: 0x0000000000000030
+; F0:             Ending address offset: 0x000000000000002c
 ; F0:              Location description: 51
----------------
Could you explain why this changed?


Repository:
  rL LLVM

https://reviews.llvm.org/D23551