[PATCH] ARM: fold stack update into push/pop

[Renato Golin]

  Hi Tim,

  If I got it right, you're detecting the first push and the last pop and is just adding clobbered registers to them to save the stack pointer arithmetic. I can't think of a situation where the first push/last pop won't be prologue/epilogue, so I'll skip the relevant comment.

  Apart from the style comments I had, I can't see anything wrong with it, but it'd be good to have someone else's comment (Jim?), too.


================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1876
@@ +1875,3 @@
+
+  bool IsPop = isPopOpcode(MI->getOpcode());
+  bool IsVFPPushPop = MI->getOpcode() == ARM::VSTMDDB_UPD ||
----------------
This seems redundant with the code above, could be moved up and used there.

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1873
@@ +1872,3 @@
+      MI->getOpcode() != ARM::STMDB_UPD &&
+      MI->getOpcode() != ARM::VSTMDDB_UPD && !isPopOpcode(MI->getOpcode()))
+    return false;
----------------
It's probably not used elsewhere, but would be consistent in having isPushOpcode() above?

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1887
@@ +1886,3 @@
+
+  if (NumBytes % (IsVFPPushPop ? 8 : 4) != 0)
+    return false;
----------------
A short comment about alignment would be welcomed here

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1895
@@ +1894,3 @@
+  // Calculate the space we'll need in terms of registers.
+  unsigned FirstReg = MI->getOperand(RegListIdx).getReg(), RD0Reg, RegsNeeded;
+  if (IsVFPPushPop) {
----------------
this line is confusing, better split in two.

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:1946
@@ +1945,3 @@
+
+  // 3. Add registers.
+  MachineInstrBuilder MIB(MF, &*MI);
----------------
3?


http://llvm-reviews.chandlerc.com/D2105