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

[Simon Dardis via Phabricator via llvm-commits]

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

LGTM with inline nits addressed.



================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1836-1851
+def ORiPred  : PatLeaf<(imm), [{
+  uint64_t ZVal = N->getZExtValue();
+  int64_t SVal = N->getSExtValue();
+  bool cond =  isUInt<16>(ZVal) && !isInt<16>(SVal);
+  return (uint32_t)ZVal == (unsigned short)ZVal && cond;
+}], LO16>;
+
----------------
These should go in the block of other leaf patterns starting at 1155.


================
Comment at: lib/Target/Mips/MipsInstrInfo.td:1839-1840
+  int64_t SVal = N->getSExtValue();
+  bool cond =  isUInt<16>(ZVal) && !isInt<16>(SVal);
+  return (uint32_t)ZVal == (unsigned short)ZVal && cond;
+}], LO16>;
----------------
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


================
Comment at: test/CodeGen/Mips/constMaterialization.ll:3
+; RUN: llc -march=mips < %s -mattr=+micromips | FileCheck %s -check-prefixes=ALL,MM
+
+; Constants generated using li16
----------------
Add a comment here explaining that this test documents/tests the patterns used for constant materialization.


================
Comment at: test/CodeGen/Mips/constMaterialization.ll:7
+entry:
+; ALL-LABEL: Li16LowBoundary:
+  ; MIPS:     addiu	$2, $zero, -1
----------------
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.


================
Comment at: test/CodeGen/Mips/constMaterialization.ll:35
+; ALL-LABEL: AddiuLowBoundary:
+  ; MIPS:     addiu	$2, $zero, -32768
+  ; ALL-NOT:  lui
----------------
This should be ALL.


================
Comment at: test/CodeGen/Mips/constMaterialization.ll:75
+  ; ALL-NOT: li16
+ 
+  ret i32 32768
----------------
Stray whitespace here and in the rest of the functions at the same location.


================
Comment at: test/CodeGen/Mips/constMaterialization.ll:90
+
+; Constants generated using lui
+define i32 @Lui() {
----------------
Check that a negative number that doesn't have the lower 16 bits is synthesized with a single lui.


https://reviews.llvm.org/D38841