[mips] Provide alternate predicates for constant synthesis

[smaksimovic]

Summary

Ordering of patterns should not be of importance anymore since the predicates used are mutually exclusive now.

[smaksimovic]

We could, in addition to the new predicates, reorder the patterns which use them resulting in exposure of the underlying dependence of the order the patterns are laid out.
I stuck with Val & 0xfff instead of suggested isUInt<16>(Val) since it seems that it behaves differently.

[sdardis]

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.

lib/Target/Mips/MicroMipsInstrInfo.td

1008–1009

Use AdditionalPredicates with a scope to cover both the def and multidef.

1010–1012

Delete these patterns, they are covered by the MaterializeImms multiclass and the explict pattern for LI16_MM above.

lib/Target/Mips/MipsInstrInfo.td

2732–2746

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?

[smaksimovic]

Addressed review comments.

[sdardis]

LGTM with inline nits addressed.

lib/Target/Mips/MipsInstrInfo.td

1850–1865

These should go in the block of other leaf patterns starting at 1155.

1853–1854

You can return the result of isUInt<16>(ZVal) && !isInt<16> directly, as isUInt<16> already tests

return static_cast<int16_t>(x) == x;

See include/llvm/Support/MathExtras.h

test/CodeGen/Mips/constMaterialization.ll

4

Add a comment here explaining that this test documents/tests the patterns used for constant materialization.

8

This check line needs to be indented to the same level as the check lines below. This applies to the rest of the functions in this test.

36

This should be ALL.

76

Stray whitespace here and in the rest of the functions at the same location.

91

Check that a negative number that doesn't have the lower 16 bits is synthesized with a single lui.

[smaksimovic]

Nits addressed.

lib/Target/Mips/MicroMipsInstrInfo.td

let DecoderNamespace = "MicroMips" in {
   def LWU_MM : MMRel, LoadMM<"lwu", GPR32Opnd, zextloadi32, II_LWU,
                              mem_simm12>, LL_FM_MM<0xe>,
                ISA_MICROMIPS32_NOT_MIPS32R6;
 }
 
 //===----------------------------------------------------------------------===//
 // MicroMips arbitrary patterns that map to one or more instructions
 //===----------------------------------------------------------------------===//
 
+let AdditionalPredicates = [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>;
+}
 
 let Predicates = [InMicroMips] in {
-  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)>;
-
   def : MipsPat<(not GPRMM16:$in),
                 (NOT16_MM GPRMM16:$in)>;
   def : MipsPat<(not GPR32:$in),
                 (NOR_MM GPR32Opnd:$in, ZERO)>;
 
   def : MipsPat<(add GPRMM16:$src, immSExtAddiur2:$imm),
                 (ADDIUR2_MM GPRMM16:$src, immSExtAddiur2:$imm)>;
   def : MipsPat<(add GPR32:$src, immSExtAddius5:$imm),

lib/Target/Mips/MipsInstrInfo.td

def immSExt16Plus1 : PatLeaf<(imm), [{
   return isInt<17>(N->getSExtValue()) && isInt<16>(N->getSExtValue() + 1);
 }]>;
 
 def immZExtRange2To64 : PatLeaf<(imm), [{
   return isUInt<7>(N->getZExtValue()) && (N->getZExtValue() >= 2) &&
          (N->getZExtValue() <= 64);
 }]>;
 
+def ORiPred  : PatLeaf<(imm), [{
+  return isUInt<16>(N->getZExtValue()) && !isInt<16>(N->getSExtValue());
+}], LO16>;
+
+def LUiPred : PatLeaf<(imm), [{
+  int64_t Val = N->getSExtValue();
+  return !isInt<16>(Val) && isInt<32>(Val) && !(Val & 0xffff);
+}]>;
+
+def LUiORiPred  : PatLeaf<(imm), [{
+  int64_t SVal = N->getSExtValue();
+  return isInt<32>(SVal) && (SVal & 0xffff);
+}]>;
+
 // Mips Address Mode! SDNode frameindex could possibily be a match
 // since load and store instructions from stack used it.
 def addr :
   ComplexPattern<iPTR, 2, "selectIntAddr", [frameindex]>;
 
 def addrRegImm :
   ComplexPattern<iPTR, 2, "selectAddrRegImm", [frameindex]>;
 

 // Expands to: lui $dst, %hi($tgt - $baltgt)
 def LONG_BRANCH_LUi : PseudoSE<(outs GPR32Opnd:$dst),
   (ins brtarget:$tgt, brtarget:$baltgt), []>;
 
 // Expands to: addiu $dst, $src, %lo($tgt - $baltgt)
 def LONG_BRANCH_ADDiu : PseudoSE<(outs GPR32Opnd:$dst),
   (ins GPR32Opnd:$src, brtarget:$tgt, brtarget:$baltgt), []>;
 
 //===----------------------------------------------------------------------===//
 // Instruction definition
 //===----------------------------------------------------------------------===//
 //===----------------------------------------------------------------------===//
 // MipsI Instructions
 //===----------------------------------------------------------------------===//
 
 /// Arithmetic Instructions (ALU Immediate)
 let AdditionalPredicates = [NotInMicroMips] in {
   def ADDiu : MMRel, StdMMR6Rel, ArithLogicI<"addiu", simm16_relaxed, GPR32Opnd,
                                              II_ADDIU, immSExt16, add>,
               ADDI_FM<0x9>, IsAsCheapAsAMove;
 
   def ANDi : MMRel, StdMMR6Rel,
              ArithLogicI<"andi", uimm16, GPR32Opnd, II_ANDI, immZExt16, and>,
              ADDI_FM<0xc>;
   def ORi  : MMRel, StdMMR6Rel,
              ArithLogicI<"ori", uimm16, GPR32Opnd, II_ORI, immZExt16, or>,
              ADDI_FM<0xd>;
   def XORi : MMRel, StdMMR6Rel,
              ArithLogicI<"xori", uimm16, GPR32Opnd, II_XORI, immZExt16, xor>,
              ADDI_FM<0xe>;
 }
 def ADDi  : MMRel, ArithLogicI<"addi", simm16_relaxed, GPR32Opnd, II_ADDI>, ADDI_FM<0x8>,

 
 class StoreRegImmPat<Instruction StoreInst, ValueType ValTy> :
   MipsPat<(store ValTy:$v, addrRegImm:$a), (StoreInst ValTy:$v, addrRegImm:$a)>;
 
 // Materialize constants.
 multiclass MaterializeImms<ValueType VT, Register ZEROReg,
                            Instruction ADDiuOp, Instruction LUiOp,
                            Instruction ORiOp> {
 
-// Small immediates
-def : MipsPat<(VT immSExt16:$imm), (ADDiuOp ZEROReg, imm:$imm)>;
-def : MipsPat<(VT immZExt16:$imm), (ORiOp ZEROReg, imm:$imm)>;
+// Constant synthesis previously relied on the ordering of the patterns below.
+// By making the predicates they use non-overlapping, the patterns were
+// reordered so that the effect of the newly introduced predicates can be
+// observed.
+
+// Arbitrary immediates
+def : MipsPat<(VT LUiORiPred:$imm), (ORiOp (LUiOp (HI16 imm:$imm)), (LO16 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))>;
 
-// Arbitrary immediates
-def : MipsPat<(VT immSExt32:$imm), (ORiOp (LUiOp (HI16 imm:$imm)), (LO16 imm:$imm))>;
+// Small immediates
+def : MipsPat<(VT ORiPred:$imm), (ORiOp ZEROReg, imm:$imm)>;
+def : MipsPat<(VT immSExt16:$imm), (ADDiuOp ZEROReg, imm:$imm)>;
 }
 
 let AdditionalPredicates = [NotInMicroMips] in
   defm : MaterializeImms<i32, ZERO, ADDiu, LUi, ORi>;
 
 // Carry MipsPatterns
 let AdditionalPredicates = [NotInMicroMips] in {
   def : MipsPat<(subc GPR32:$lhs, GPR32:$rhs),

test/CodeGen/Mips/constMaterialization.ll

+; RUN: llc -march=mips < %s | FileCheck %s -check-prefixes=ALL,MIPS
+; RUN: llc -march=mips < %s -mattr=+micromips | FileCheck %s -check-prefixes=ALL,MM
+
+; Test the patterns used for constant materialization.
+
+; Constants generated using li16
+define i32 @Li16LowBoundary() {
+entry:
+  ; ALL-LABEL: Li16LowBoundary:
+  ; MIPS:     addiu $2, $zero, -1
+  ; MM:       li16  $2, -1
+  ; ALL-NOT:  lui
+  ; ALL-NOT:  ori
+  ; MIPS-NOT: li16
+  ; MM-NOT:   addiu
+
+  ret i32 -1
+}
+
+define i32 @Li16HighBoundary() {
+entry:
+  ; ALL-LABEL: Li16HighBoundary:
+  ; MIPS:     addiu $2, $zero, 126
+  ; MM:       li16  $2, 126
+  ; ALL-NOT:  lui
+  ; ALL-NOT:  ori
+  ; MM-NOT:   addiu
+  ; MIPS-NOT: li16
+
+  ret i32 126
+}
+
+; Constants generated using addiu
+define i32 @AddiuLowBoundary() {
+entry:
+  ; ALL-LABEL: AddiuLowBoundary:
+  ; ALL:      addiu $2, $zero, -32768
+  ; ALL-NOT:  lui
+  ; ALL-NOT:  ori
+  ; ALL-NOT:  li16
+
+  ret i32 -32768
+}
+
+define i32 @AddiuZero() {
+entry:
+  ; ALL-LABEL: AddiuZero:
+  ; MIPS:     addiu $2, $zero, 0
+  ; MM:       li16  $2, 0
+  ; ALL-NOT:  lui
+  ; ALL-NOT:  ori
+  ; MIPS-NOT: li16
+  ; MM-NOT:   addiu
+
+  ret i32 0
+}
+
+define i32 @AddiuHighBoundary() {
+entry:
+  ; ALL-LABEL: AddiuHighBoundary:
+  ; ALL:     addiu  $2, $zero, 32767
+  ; ALL-NOT: lui
+  ; ALL-NOT: ori
+  ; ALL-NOT: li16
+
+  ret i32 32767
+}
+
+; Constants generated using ori
+define i32 @OriLowBoundary() {
+entry:
+  ; ALL-LABEL: OriLowBoundary:
+  ; ALL:     ori  $2, $zero, 32768
+  ; ALL-NOT: addiu
+  ; ALL-NOT: lui
+  ; ALL-NOT: li16
+
+  ret i32 32768
+}
+
+define i32 @OriHighBoundary() {
+entry:
+  ; ALL-LABEL: OriHighBoundary:
+  ; ALL:     ori  $2, $zero, 65535
+  ; ALL-NOT: addiu
+  ; ALL-NOT: lui
+  ; ALL-NOT: li16
+
+  ret i32 65535
+}
+
+; Constants generated using lui
+define i32 @LuiPositive() {
+entry:
+  ; ALL-LABEL: LuiPositive:
+  ; ALL:     lui  $2, 1
+  ; ALL-NOT: addiu
+  ; ALL-NOT: ori
+  ; ALL-NOT: li16
+
+  ret i32 65536
+}
+
+define i32 @LuiNegative() {
+entry:
+  ; ALL-LABEL: LuiNegative:
+  ; ALL:     lui  $2, 65535
+  ; ALL-NOT: addiu
+  ; ALL-NOT: ori
+  ; ALL-NOT: li16
+
+  ret i32 -65536
+}
+
+; Constants generated using a combination of lui and ori
+define i32 @LuiWithLowBitsSet() {
+entry:
+  ; ALL-LABEL: LuiWithLowBitsSet:
+  ; ALL:     lui  $1, 1
+  ; ALL:     ori  $2, $1, 1
+  ; ALL-NOT: addiu
+  ; ALL-NOT: li16
+
+  ret i32 65537
+}
+
+define i32 @BelowAddiuLowBoundary() {
+entry:
+  ; ALL-LABEL: BelowAddiuLowBoundary:
+  ; ALL:     lui  $1, 65535
+  ; ALL:     ori  $2, $1, 32767
+  ; ALL-NOT: addiu
+  ; ALL-NOT: li16
+
+  ret i32 -32769
+}