Fix ARM assembler to pop registers in reverse order of pushes.

src/IceInstARM32.cpp

Index: src/IceInstARM32.cpp
diff --git a/src/IceInstARM32.cpp b/src/IceInstARM32.cpp
index 64bdefa7224e75de7c8e4ed6c62aba0cc85d5564..1b459bbf412c72c46ab769e2a9194968d43a60df 100644
--- a/src/IceInstARM32.cpp
+++ b/src/IceInstARM32.cpp
@@ -898,17 +898,6 @@ void InstARM32RegisterStackOp::emitSRegsAsText(const Cfg *Func,
   Str << "}";
 }
 
-namespace {
-
-bool isAssignedConsecutiveRegisters(const Variable *Before,
-                                    const Variable *After) {
-  assert(Before->hasReg());
-  assert(After->hasReg());
-  return Before->getRegNum() + 1 == After->getRegNum();
-}
-
-} // end of anonymous namespace
-
 void InstARM32RegisterStackOp::emitUsingForm(const Cfg *Func,
                                              const EmitForm Form) const {
   SizeT NumRegs = getNumStackRegs();
@@ -938,12 +927,14 @@ void InstARM32RegisterStackOp::emitUsingForm(const Cfg *Func,
     const Variable *BaseReg = nullptr;
     SizeT RegCount = 0;
     for (SizeT i = 0; i < NumRegs; ++i) {
-      const Variable *NextReg = getStackReg(i);
+      const Variable *NextReg = getStackSReg(i, NumRegs);
+      assert(NextReg->hasReg());
       if (BaseReg == nullptr) {
         BaseReg = NextReg;
         RegCount = 1;
       } else if (RegCount < VpushVpopMaxConsecRegs &&
                  isAssignedConsecutiveRegisters(Reg, NextReg)) {
+        BaseReg = updateBaseReg(BaseReg, NextReg);
         ++RegCount;
       } else {
         emitSRegs(Func, Form, BaseReg, RegCount);
@@ -1780,6 +1771,21 @@ Variable *InstARM32Pop::getStackReg(SizeT Index) const { return Dests[Index]; }
 
 SizeT InstARM32Pop::getNumStackRegs() const { return Dests.size(); }
 
+bool InstARM32Pop::isAssignedConsecutiveRegisters(const Variable *Before,
+                                                  const Variable *After) const {
+  return Before->getRegNum() - 1 == After->getRegNum();

[Jim Stichnoth, 2016/02/05 00:48:17]

I realize this is just transplanted code, but...

This code is relying on a certain ordering of the register enum values as laid
out in the .def file.  I want to be able to change/scramble the order and have
the generated code continue to work (though not necessarily being precisely as
efficient).

I think to be safe here, you should actually be comparing the encoded values,
not the enum values.

[John, 2016/02/05 01:46:25]

The only issue with encoded reg values (that is also a problem with the current
implementation) is that it will not catch resister type mix ups. But agreed,
using the encoded reg value is better.

[Karl, 2016/02/05 17:00:05]

Done.

+}
+
+Variable *InstARM32Pop::getStackSReg(SizeT Index, SizeT Size) const {

[John, 2016/02/05 01:46:25]

Do you need to pass Size here? Isn't size the same as Dest.size()? If so, just
invoke it here

[Karl, 2016/02/05 17:00:05]

No longer needed. Removed need for virtuals.

+  return getStackReg(Size - (Index + 1));
+}
+
+const Variable *InstARM32Pop::updateBaseReg(const Variable *BaseReg,
+                                            const Variable *NextReg) const {
+  (void)BaseReg;
+  return NextReg;
+}
+
 void InstARM32Pop::emitSingleGPR(const Cfg *Func, const EmitForm Form,
                                  const Variable *Reg) const {
   switch (Form) {
@@ -1828,6 +1834,22 @@ Variable *InstARM32Push::getStackReg(SizeT Index) const {
 
 SizeT InstARM32Push::getNumStackRegs() const { return getSrcSize(); }
 
+bool InstARM32Push::isAssignedConsecutiveRegisters(
+    const Variable *Before, const Variable *After) const {
+  return Before->getRegNum() + 1 == After->getRegNum();
+}
+
+Variable *InstARM32Push::getStackSReg(SizeT Index, SizeT Size) const {
+  (void)Size;
+  return getStackReg(Index);
+}
+
+const Variable *InstARM32Push::updateBaseReg(const Variable *BaseReg,
+                                             const Variable *NextReg) const {
+  (void)NextReg;
+  return BaseReg;
+}
+
 void InstARM32Push::emitSingleGPR(const Cfg *Func, const EmitForm Form,
                                   const Variable *Reg) const {
   switch (Form) {