[PATCH] D47721: [X86] Fix skylake server scheduling info.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Nice! A couple of minors - but almost there I reckon.



================
Comment at: lib/Target/X86/X86SchedSkylakeServer.td:337
+defm : SKXWriteResPair<WritePSADBWZ, [SKXPort5], 3, [1], 1, 7>;
 defm : SKXWriteResPair<WritePHMINPOS, [SKXPort015], 4, [1], 1, 6>; // Vector PHMINPOS.
 
----------------
You results suggest WritePHMINPOS should be just Port0?


================
Comment at: lib/Target/X86/X86SchedSkylakeServer.td:378
 // Conversion between integer and float.
-defm : SKXWriteResPair<WriteCvtSS2I,   [SKXPort0,SKXPort015], 6, [1,1], 2>;
-defm : SKXWriteResPair<WriteCvtPS2I,   [SKXPort1], 3>;
-defm : SKXWriteResPair<WriteCvtPS2IY,  [SKXPort1], 3>;
-defm : SKXWriteResPair<WriteCvtSD2I,   [SKXPort0,SKXPort015], 6, [1,1], 2>;
-defm : SKXWriteResPair<WriteCvtPD2I,   [SKXPort1], 3>;
-defm : SKXWriteResPair<WriteCvtPD2IY,  [SKXPort1], 3>;
+defm : SKXWriteResPair<WriteCvtSS2I,   [SKXPort01], 6, [2], 2>; // Needs more work: DD vs DQ.
+defm : SKXWriteResPair<WriteCvtPS2I,   [SKXPort01], 3>;
----------------
The QQ conversions might be best off with instrw overrides?


================
Comment at: lib/Target/X86/X86SchedSkylakeServer.td:390
+defm : SKXWriteResPair<WriteCvtI2PSY,  [SKXPort01], 4>;
+defm : SKXWriteResPair<WriteCvtI2PSZ,  [SKXPort05], 4>;  // Needs more work: DD vs DQ.
 defm : SKXWriteResPair<WriteCvtI2SD,   [SKXPort1], 4>;
----------------
The QQ conversions might be best off with instrw overrides?


================
Comment at: lib/Target/X86/X86SchedSkylakeServer.td:406
+defm : X86WriteRes<WriteCvtPH2PS,     [SKXPort5,SKXPort01],  5, [1,1], 2>;
+defm : X86WriteRes<WriteCvtPH2PSY,    [SKXPort5,SKXPort01],  7, [1,1], 2>;  // Needs more work.
+defm : X86WriteRes<WriteCvtPH2PSZ,    [SKXPort5,SKXPort0],   7, [1,1], 2>;
----------------
WriteCvtPH2PSY is the one that does pass your tests - it's WriteCvtPH2PS and WriteCvtPH2PSZ that are irregular?


================
Comment at: lib/Target/X86/X86SchedSkylakeServer.td:609
 }
-def: InstRW<[SKXWriteResGroup7], (instrs CDQ, CQO, CLAC, STAC)>;
+def: InstRW<[SKXWriteResGroup7], (instrs CDQ, CQO, CLAC, STAC, LAHF, SAHF)>;
 def: InstRW<[SKXWriteResGroup7], (instregex "BT(16|32|64)ri8",
----------------
Worth pulling the LAHF/SAHF change out into its own patch as IIRC a lot of Intel models have dodgy values for this?


Repository:
  rL LLVM

https://reviews.llvm.org/D47721