[PATCH] D38841: [mips] Provide alternate predicates for constant synthesis

Simon Dardis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 12 03:55:22 PDT 2017


sdardis added a comment.

Some quick inlined comments.

Can you provide a test case where you check the various edge values and use a combination of CHECK and CHECK-NOT to assert that the ISel is picking the expected instructions?

E.g.

  define i32 @ret_zero() {
    CHECK-MIPS:     addiu $2, $zero, 0
    CHECK-MIPS-NOT: lui
    CHECK-MIPS-NOT: ori
    CHECK-MIPS-NOT: li16
   
    ret i32 0
  }

The goal of the test case is to document what's "canonical" for us in terms of constant materialisation or at least the current behaviour.



================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:1009-1015
+let Predicates = [InMicroMips] in {
+  def : MipsPat<(i32 immLi16:$imm),
+                (LI16_MM immLi16:$imm)>;
+}
 
 let AdditionalPredicates = [InMicroMips] in
 defm :  MaterializeImms<i32, ZERO, ADDiu_MM, LUi_MM, ORi_MM>;
----------------
Use AdditionalPredicates with a scope to cover both the def and multidef.


================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:1018-1023
   def : MipsPat<(i32 immLi16:$imm),
                 (LI16_MM immLi16:$imm)>;
   def : MipsPat<(i32 immSExt16:$imm),
                 (ADDiu_MM ZERO, immSExt16:$imm)>;
   def : MipsPat<(i32 immZExt16:$imm),
                 (ORi_MM ZERO, immZExt16:$imm)>;
----------------
Delete these patterns, they are covered by the MaterializeImms multiclass and the explict pattern for LI16_MM above.


================
Comment at: lib/Target/Mips/MipsInstrInfo.td:2736-2744
 // Small immediates
 def : MipsPat<(VT immSExt16:$imm), (ADDiuOp ZEROReg, imm:$imm)>;
-def : MipsPat<(VT immZExt16:$imm), (ORiOp ZEROReg, imm:$imm)>;
+def : MipsPat<(VT ORiPred:$imm), (ORiOp ZEROReg, imm:$imm)>;
 
 // Bits 32-16 set, sign/zero extended.
-def : MipsPat<(VT immSExt32Low16Zero:$imm), (LUiOp (HI16 imm:$imm))>;
+def : MipsPat<(VT LUiPred:$imm), (LUiOp (HI16 imm:$imm))>;
 
----------------
Your suggestion for reordering the predicates is a good one, can you invert the order and add a comment saying why they are in an unusual order?


https://reviews.llvm.org/D38841





More information about the llvm-commits mailing list