[PATCH] D21321: [mips] Optimize stack pointer adjustments.

[Daniel Sanders via llvm-commits]

dsanders added subscribers: nitesh.jain, mohit.bhakkad, slthakur, bhushan.
dsanders added a comment.

Added Nitesh, Mohit, Sagar, and Bhushan in case this requires changes to lldb.

Just one question about a fixme and a few minor nits.


================
Comment at: lib/Target/Mips/MipsSEInstrInfo.cpp:458-461
@@ -457,4 +457,6 @@
 
-  if (isInt<16>(Amount))// addi sp, sp, amount
+  if (isInt<16>(Amount))
+    // addi sp, sp, amount
     BuildMI(MBB, I, DL, get(ADDiu), SP).addReg(SP).addImm(Amount);
-  else { // Expand immediate that doesn't fit in 16-bit.
+  else {
+    // For numbers which are not 16bit integers we synthesize Amount inline
----------------
Could you put braces around this now that it's multiple lines?

================
Comment at: lib/Target/Mips/MipsSEInstrInfo.cpp:469-470
@@ -461,3 +468,4 @@
+    }
     unsigned Reg = loadImmediate(Amount, MBB, I, DL, nullptr);
-    BuildMI(MBB, I, DL, get(ADDu), SP).addReg(SP).addReg(Reg, RegState::Kill);
+    BuildMI(MBB, I, DL, get(Opc), SP).addReg(SP).addReg(Reg, RegState::Kill);
   }
----------------
These suggestions are for another patch but just to mention them:
* MIPS32R6/MIPS64R6 can add the immediate without materializing it first using AUI/DAUI/DATI/DAHI.
* MIPS32R5/MIPS64R5 with MSA, and MIPS32R6/MIPS64R6 can improve on this using LSA/DLSA to add 17-20 bit immediates in two instructions instead of three as long as the amount is appropriately aligned (which is always true for 17-19 bit, and true on N32/N64 for 20-bit).

================
Comment at: test/CodeGen/Mips/cstmaterialization/stack.ll:2-3
@@ +1,4 @@
+; RUN: llc -march=mipsel -mcpu=mips32 < %s | FileCheck %s -check-prefix=CHECK-MIPS32
+; RUN: llc -march=mips64el -mcpu=mips64 < %s | \
+; RUN:      FileCheck %s -check-prefix=CHECK-MIPS64
+
----------------
Could you add the N32 case?

================
Comment at: test/CodeGen/Mips/cstmaterialization/stack.ll:29-31
@@ +28,5 @@
+
+; FIXME:
+; These are here to match other lui's used in offset computations and they're
+; also duplicated.
+
----------------
Can you clarify what needs fixing here? Is it just the duplication or is there something else?

================
Comment at: test/CodeGen/Mips/eh-dwarf-cfa.ll:16
@@ -15,1 +15,3 @@
 
+; CHECK-LABEL: f1
+
----------------
Could you add a colon to each of these to reduce the chance of an accidental match on something like $f1 or 0xf1?

================
Comment at: test/CodeGen/Mips/largeimm1.ll:9
@@ -10,1 +8,3 @@
   ret void
+; CHECK-LABEL: f
+
----------------
This will match the 'f' in '.file ...' instead of the function label 'f:'


Repository:
  rL LLVM

http://reviews.llvm.org/D21321