[PATCH] D40067: [X86] Update BTVER2 sched numbers for some AVX instructions (mmx version)

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

We're adding a lot of instregex exntries just to get around the poor quality of the sched classes in X86Schedule.td - better to fix them?



================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:578
 
-def WriteVMONTPSt: SchedWriteRes<[JSTC, JLAGU]> {
+def WriteVMOVTDQSt: SchedWriteRes<[JSTC, JLAGU]> {
+  let Latency = 2;
----------------
Shouldn't stores (mr) have a JSAGU dependency not a JLAGU


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:583
+
+def WriteVMOVTPSt: SchedWriteRes<[JSTC, JLAGU]> {
+  let Latency = 3;
----------------
Shouldn't stores (mr) have a JSAGU dependency not a JLAGU


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:588
+
+def WriteVMONTPYSt: SchedWriteRes<[JSTC, JLAGU]> {
   let Latency = 3;
----------------
Shouldn't stores (mr) have a JSAGU dependency not a JLAGU?


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:596
+def WriteVMOV: SchedWriteRes<[JFPU01]> {
+  let ResourceCycles = [1];
+}
----------------
Remove ResourceCyles? Looks like the default.


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:598
+}
+def : InstRW<[WriteVMOV], (instregex "VMOVS(S|D)rr", "VMOVS(S|D)mr")>;
+def : InstRW<[WriteVMOV], (instregex "VMOVUPDrr", "VMOVUPDmr")>;
----------------
I wonder if we'd be better off introducing WriteVecLoad/WriteVecStore/WriteVecMove sched rw classes like the FIXME comment at line 140 suggests?


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:602
+def : InstRW<[WriteVMOV], (instregex "VMOVDQArr", "VMOVDQAmr")>;
+def : InstRW<[WriteVMOV], (instregex "VMOVDQUrr", "VMOVDQUmr")>;
+
----------------
Shouldn't stores (mr) have a JSAGU dependency?


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:604
+
+def WriteVMOVLd: SchedWriteRes<[JFPU01, JLAGU]> {
+  let Latency = 6;
----------------
Should JLAGU be first for a load?


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:606
+  let Latency = 6;
+  let ResourceCycles = [1, 1];
+}
----------------
Remove ResourceCyles? Looks like the default.


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:624
+def : InstRW<[WriteVMAXMINLd], (instregex "VMAXP(D|S)rm", "VMAXS(D|S)rm")>;
+def : InstRW<[WriteVMAXMINLd], (instregex "VMINP(D|S)rm", "VMINS(D|S)rm")>;
 
----------------
Would we be better off introducing a WriteFCmp sched rw class to cover max/min and cmp?


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:674
+def : InstRW<[WriteVCMPcc], (instregex "VCMPP(S|D)rri", "VCMPS(S|D)rri")>;
+//def : InstRW<[WriteVCMPcc], (instregex "VCMP..P(S|D)rr", "VCMP..S(S|D)rr")>;
+
----------------
Don't include commented out lines.


================
Comment at: lib/Target/X86/X86ScheduleBtVer2.td:680
+def : InstRW<[WriteVCMPccLd, ReadAfterLd], (instregex "VCMPP(S|D)rmi", "VCMPS(S|D)rmi")>;
+//def : InstRW<[WriteVCMPccLd, ReadAfterLd], (instregex "VCMP..P(S|D)rm", "VCMP..S(S|D)rm")>;
+
----------------
Don't include commented out lines.


https://reviews.llvm.org/D40067