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

src/IceInstARM32.cpp

 //===- subzero/src/IceInstARM32.cpp - ARM32 instruction implementation ----===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 880 matching lines @@
   for (SizeT i = 0; i < RegCount; ++i) {
     if (IsFirst)
       IsFirst = false;
     else
       Str << ", ";
     Str << RegARM32::getRegName(Base + i);
   }
   Str << "}";
 }
 
+void InstARM32RegisterStackOp::emitSRegsOp(const Cfg *Func, EmitForm Form,
+                                           const Variable *BaseReg,
+                                           SizeT RegCount,
+                                           SizeT InstIndex) const {
+  if (Form == Emit_Text && BuildDefs::dump() && InstIndex > 0) {
+    startNextInst(Func);
+    Func->getContext()->getStrEmit() << "\n";
+  }
+  emitSRegs(Func, Form, BaseReg, RegCount);
+}
+
 namespace {
 
 bool isAssignedConsecutiveRegisters(const Variable *Before,
                                     const Variable *After) {
-  assert(Before->hasReg());
-  assert(After->hasReg());
-  return Before->getRegNum() + 1 == After->getRegNum();
+  return RegARM32::getEncodedSReg(Before->getRegNum()) + 1 ==
+         RegARM32::getEncodedSReg(After->getRegNum());
 }
 
 } // end of anonymous namespace
 
 void InstARM32RegisterStackOp::emitUsingForm(const Cfg *Func,
                                              const EmitForm Form) const {
   SizeT NumRegs = getNumStackRegs();
   assert(NumRegs);
 
   const auto *Reg = llvm::cast<Variable>(getStackReg(0));
   if (isScalarIntegerType(Reg->getType())) {
-    // Pop GPR registers.
+    // Push/pop GPR registers.
     SizeT IntegerCount = 0;
     ARM32::IValueT GPRegisters = 0;
     const Variable *LastDest = nullptr;
     for (SizeT i = 0; i < NumRegs; ++i) {
       const Variable *Var = getStackReg(i);
       assert(Var->hasReg() && "stack op only applies to registers");
       int32_t Reg = RegARM32::getEncodedGPR(Var->getRegNum());
       LastDest = Var;
       GPRegisters |= (1 << Reg);
       ++IntegerCount;
     }
     if (IntegerCount == 1) {
       emitSingleGPR(Func, Form, LastDest);
     } else {
       emitMultipleGPRs(Func, Form, GPRegisters);
     }
-  } else {
-    // Pop vector/floating point registers.
-    const Variable *BaseReg = nullptr;
-    SizeT RegCount = 0;
-    for (SizeT i = 0; i < NumRegs; ++i) {
-      const Variable *NextReg = getStackReg(i);
-      if (BaseReg == nullptr) {
-        BaseReg = NextReg;
-        RegCount = 1;
-      } else if (RegCount < VpushVpopMaxConsecRegs &&
-                 isAssignedConsecutiveRegisters(Reg, NextReg)) {
-        ++RegCount;
-      } else {
-        emitSRegs(Func, Form, BaseReg, RegCount);
-        if (Form == Emit_Text && BuildDefs::dump()) {
-          startNextInst(Func);
-          Func->getContext()->getStrEmit() << "\n";
-        }
-        BaseReg = NextReg;
-        RegCount = 1;
-      }
-      Reg = NextReg;
+    return;
+  }
+
+  // Push/pop floating point registers. Divide into a list of instructions,
+  // defined on consecutive register ranges. Then generate the corresponding
+  // instructions.
+  llvm::SmallVector<std::pair<const Variable *, SizeT>, 5> InstData;

[Jim Stichnoth, 2016/02/08 17:15:58]

Could you document the choice of "5"?  With a constexpr and/or comment perhaps
like:

  // Typical max number of registers pushed/popped is no more than 5.

[Karl, 2016/02/09 20:24:27]

Done.

+  const Variable *BaseReg = nullptr;
+  SizeT RegCount = 0;
+  for (SizeT i = 0; i < NumRegs; ++i) {
+    const Variable *NextReg = getStackReg(i);
+    assert(NextReg->hasReg());
+    if (BaseReg == nullptr) {
+      BaseReg = NextReg;
+      RegCount = 1;
+    } else if (RegCount < VpushVpopMaxConsecRegs &&
+               isAssignedConsecutiveRegisters(Reg, NextReg)) {
+      ++RegCount;
+    } else {
+      InstData.push_back(std::make_pair(BaseReg, RegCount));

[Jim Stichnoth, 2016/02/08 17:15:58]

InstData.emplace_back(BaseReg, RegCount);

[Karl, 2016/02/09 20:24:27]

Done.

+      BaseReg = NextReg;
+      RegCount = 1;
     }
-    if (RegCount) {
-      emitSRegs(Func, Form, BaseReg, RegCount);
-    }
+    Reg = NextReg;
   }
+  if (RegCount) {
+    InstData.push_back(std::make_pair(BaseReg, RegCount));

[Jim Stichnoth, 2016/02/08 17:15:58]

emplace_back

[Karl, 2016/02/09 20:24:27]

Done.

+  }
+  SizeT InstCount = 0;
+  if (llvm::isa<InstARM32Push>(*this)) {
+    for (const auto &Pair : InstData)
+      emitSRegsOp(Func, Form, Pair.first, Pair.second, InstCount++);
+    return;
+  }
+  assert(llvm::isa<InstARM32Pop>(*this));
+  for (const auto &Pair : reverse_range(InstData))
+    emitSRegsOp(Func, Form, Pair.first, Pair.second, InstCount++);
 }
 
 InstARM32Pop::InstARM32Pop(Cfg *Func, const VarList &Dests)
     : InstARM32RegisterStackOp(Func, InstARM32::Pop, 0, nullptr), Dests(Dests) {
   // Track modifications to Dests separately via FakeDefs. Also, a pop
   // instruction affects the stack pointer and so it should not be allowed to
   // be automatically dead-code eliminated. This is automatic since we leave
   // the Dest as nullptr.
   validatePushOrPopRegisterListOrDie(Dests);
 }
@@ skipping 1567 matching lines @@
 
 template class InstARM32FourAddrGPR<InstARM32::Mla>;
 template class InstARM32FourAddrGPR<InstARM32::Mls>;
 
 template class InstARM32CmpLike<InstARM32::Cmn>;
 template class InstARM32CmpLike<InstARM32::Cmp>;
 template class InstARM32CmpLike<InstARM32::Tst>;
 
 } // end of namespace ARM32
 } // end of namespace Ice