[PATCH] D17198: [mips] MIPSR6 Instruction itineraries

[Vasileios Kalintiris via llvm-commits]

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

LGTM with few small changes, the most important being the addition of the entries missing from the MipsGenericItineraries table.


================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:248
@@ -247,2 +247,3 @@
 class PCREL_DESC_BASE<string instr_asm, RegisterOperand GPROpnd,
-                      Operand ImmOpnd> : MipsR6Arch<instr_asm> {
+                      InstrItinClass itin, Operand ImmOpnd>
+      : MipsR6Arch<instr_asm> {
----------------
Can we move the InstrItinClass parameter at the of the parameter list, in order to have the instruction's operands next to each other? Similarly for the 2-3 cases below.

================
Comment at: lib/Target/Mips/MipsSchedule.td:39
@@ -33,2 +38,3 @@
 def II_BBIT             : InstrItinClass; // bbit[01], bbit[01]32
+def II_BALC             : InstrItinClass;
 def II_BC               : InstrItinClass;
----------------
We don't have an entry for this in the MipsGenericItineraries table. Similarly for II_BCCC below.

================
Comment at: test/CodeGen/Mips/divrem.ll:158
@@ -157,4 +157,3 @@
 ; NOCHECK-NOT:   teq
-; GPR32:         sw $[[R0]], 0(${{[0-9]+}})
-; GPR32-DAG:     div $2, $4, $5
+; GPR32:         mod $[[R0:[0-9]+]], $4, $5
 ; GPR32-TRAP:    teq $5, $zero, 7
----------------
I think that we have to use -DAG here as the order of the div/mod instructions doesn't require a strict order. Similarly for the cases below.

================
Comment at: test/CodeGen/Mips/llvm-ir/mul.ll:172-175
@@ -171,4 +171,6 @@
 
+  ; 32R6:       mul     $3, $5, $7
+  ; 32R6:       mul     $[[T2:[0-9]+]], $4, $7
   ; 32R6:       mul     $[[T0:[0-9]+]], $5, $6
   ; 32R6:       muhu    $[[T1:[0-9]+]], $5, $7
   ; 32R6:       addu    $[[T0]], $[[T1]], $[[T0]]
----------------
We should use the -DAG suffix for these. Also, can you rename the temporaries in order to have an increasing numbering. Similarly, for 64R6 below.


http://reviews.llvm.org/D17198