Further tighten AArch32 push/pop selection.dev/push-pop

Notably:

- Check IT block status for pops that branch (by loading to pc).
- Enforce register selection rules regarding sp, lr and pc. These differ
  between T32 and A32, and sometimes between encodings.
- Relax restrictions for A1 pop (multiple); a single register is
  permitted here.

Change-Id: Ic5fe640c957652dfe2deb9d244de07a9460f1c1c

4 files changed, 98 insertions, 68 deletions

diff --git a/src/aarch32/assembler-aarch32.cc b/src/aarch32/assembler-aarch32.cc
index 9c2aaca6..fb153cd5 100644
--- a/src/aarch32/assembler-aarch32.cc
+++ b/src/aarch32/assembler-aarch32.cc
@@ -8473,27 +8473,33 @@ void Assembler::pop(Condition cond, EncodingSize size, RegisterList registers) {
   CheckIT(cond);
   if (!registers.IsEmpty() || AllowUnpredictable()) {
     if (IsUsingT32()) {
-      // POP{<c>}{<q>} <registers> ; T1
-      if (!size.IsWide() && registers.IsR0toR7orPC()) {
-        EmitT32_16(0xbc00 | (GetRegisterListEncoding(registers, 15, 1) << 8) |
-                   GetRegisterListEncoding(registers, 0, 8));
-        AdvanceIT();
-        return;
-      }
-      // POP{<c>}{<q>} <registers> ; T2
-      if (!size.IsNarrow() && !registers.Includes(sp) &&
-          (!registers.IsSingleRegister() || AllowUnpredictable())) {
-        EmitT32_32(0xe8bd0000U |
-                   (GetRegisterListEncoding(registers, 15, 1) << 15) |
-                   (GetRegisterListEncoding(registers, 14, 1) << 14) |
-                   GetRegisterListEncoding(registers, 0, 13));
-        AdvanceIT();
-        return;
+      // A branch out of an IT block should be the last instruction in the
+      // block.
+      if (!registers.Includes(pc) || OutsideITBlockAndAlOrLast(cond) ||
+          AllowUnpredictable()) {
+        // POP{<c>}{<q>} <registers> ; T1
+        if (!size.IsWide() && registers.IsR0toR7orPC()) {
+          EmitT32_16(0xbc00 | (GetRegisterListEncoding(registers, 15, 1) << 8) |
+                     GetRegisterListEncoding(registers, 0, 8));
+          AdvanceIT();
+          return;
+        }
+        // POP{<c>}{<q>} <registers> ; T2
+        // Alias of: LDM{<c>}{<q>} SP!, <registers> ; T2
+        if (!size.IsNarrow() &&
+            ((!registers.Includes(sp) && (registers.GetCount() > 1) &&
+              !(registers.Includes(pc) && registers.Includes(lr))) ||
+             AllowUnpredictable())) {
+          EmitT32_32(0xe8bd0000U | GetRegisterListEncoding(registers, 0, 16));
+          AdvanceIT();
+          return;
+        }
       }
     } else {
       // POP{<c>}{<q>} <registers> ; A1
+      // Alias of: LDM{<c>}{<q>} SP!, <registers> ; A1
       if (cond.IsNotNever() &&
-          (!registers.IsSingleRegister() || AllowUnpredictable())) {
+          (!registers.Includes(sp) || AllowUnpredictable())) {
         EmitA32(0x08bd0000U | (cond.GetCondition() << 28) |
                 GetRegisterListEncoding(registers, 0, 16));
         return;
@@ -8506,19 +8512,24 @@ void Assembler::pop(Condition cond, EncodingSize size, RegisterList registers) {
 void Assembler::pop(Condition cond, EncodingSize size, Register rt) {
   VIXL_ASSERT(AllowAssembler());
   CheckIT(cond);
-  if (IsUsingT32()) {
-    // POP{<c>}{<q>} <single_register_list> ; T4
-    if (!size.IsNarrow() && ((!rt.IsPC() || OutsideITBlockAndAlOrLast(cond)) ||
-                             AllowUnpredictable())) {
-      EmitT32_32(0xf85d0b04U | (rt.GetCode() << 12));
-      AdvanceIT();
-      return;
-    }
-  } else {
-    // POP{<c>}{<q>} <single_register_list> ; A1
-    if (cond.IsNotNever()) {
-      EmitA32(0x049d0004U | (cond.GetCondition() << 28) | (rt.GetCode() << 12));
-      return;
+  if (!rt.IsSP() || AllowUnpredictable()) {
+    if (IsUsingT32()) {
+      // POP{<c>}{<q>} <single_register_list> ; T4
+      // Alias of: LDR{<c>}{<q>} <Rt>, [SP], #4 ; T4
+      if (!size.IsNarrow() && (!rt.IsPC() || OutsideITBlockAndAlOrLast(cond) ||
+                               AllowUnpredictable())) {
+        EmitT32_32(0xf85d0b04U | (rt.GetCode() << 12));
+        AdvanceIT();
+        return;
+      }
+    } else {
+      // POP{<c>}{<q>} <single_register_list> ; A1
+      // Alias of: LDR{<c>}{<q>} <Rt>, [SP], #4 ; T1
+      if (cond.IsNotNever()) {
+        EmitA32(0x049d0004U | (cond.GetCondition() << 28) |
+                (rt.GetCode() << 12));
+        return;
+      }
     }
   }
   Delegate(kPop, &Assembler::pop, cond, size, rt);
@@ -8539,18 +8550,23 @@ void Assembler::push(Condition cond,
         return;
       }
       // PUSH{<c>}{<q>} <registers> ; T1
-      if (!size.IsNarrow() && !registers.Includes(sp) &&
-          !registers.Includes(pc) &&
-          (!registers.IsSingleRegister() || AllowUnpredictable())) {
-        EmitT32_32(0xe92d0000U |
-                   (GetRegisterListEncoding(registers, 14, 1) << 14) |
-                   GetRegisterListEncoding(registers, 0, 13));
+      // Alias of: STMDB SP!, <registers> ; T1
+      if (!size.IsNarrow() && !registers.Includes(pc) &&
+          ((!registers.Includes(sp) && (registers.GetCount() > 1)) ||
+           AllowUnpredictable())) {
+        EmitT32_32(0xe92d0000U | GetRegisterListEncoding(registers, 0, 15));
         AdvanceIT();
         return;
       }
     } else {
       // PUSH{<c>}{<q>} <registers> ; A1
-      if (cond.IsNotNever()) {
+      // Alias of: STMDB SP!, <registers> ; A1
+      if (cond.IsNotNever() &&
+          // For A32, sp can appear in the list, but stores an UNKNOWN value if
+          // it is not the lowest-valued register.
+          (!registers.Includes(sp) ||
+           registers.GetFirstAvailableRegister().IsSP() ||
+           AllowUnpredictable())) {
         EmitA32(0x092d0000U | (cond.GetCondition() << 28) |
                 GetRegisterListEncoding(registers, 0, 16));
         return;
@@ -8565,14 +8581,17 @@ void Assembler::push(Condition cond, EncodingSize size, Register rt) {
   CheckIT(cond);
   if (IsUsingT32()) {
     // PUSH{<c>}{<q>} <single_register_list> ; T4
-    if (!size.IsNarrow() && (!rt.IsPC() || AllowUnpredictable())) {
+    // Alias of: STR{<c>}{<q>} <Rt>, [SP, #4]! ; T4
+    if (!size.IsNarrow() &&
+        ((!rt.IsPC() && !rt.IsSP()) || AllowUnpredictable())) {
       EmitT32_32(0xf84d0d04U | (rt.GetCode() << 12));
       AdvanceIT();
       return;
     }
   } else {
     // PUSH{<c>}{<q>} <single_register_list> ; A1
-    if (cond.IsNotNever() && (!rt.IsPC() || AllowUnpredictable())) {
+    // Alias of: STR{<c>}{<q>} <Rt>, [SP, #4]! ; A1
+    if (cond.IsNotNever() && (!rt.IsSP() || AllowUnpredictable())) {
       EmitA32(0x052d0004U | (cond.GetCondition() << 28) | (rt.GetCode() << 12));
       return;
     }
diff --git a/src/aarch32/instructions-aarch32.h b/src/aarch32/instructions-aarch32.h
index 29272263..ff97d55e 100644
--- a/src/aarch32/instructions-aarch32.h
+++ b/src/aarch32/instructions-aarch32.h
@@ -492,6 +492,7 @@ class RegisterList {
   Register GetFirstAvailableRegister() const;
   bool IsEmpty() const { return list_ == 0; }
   bool IsSingleRegister() const { return IsPowerOf2(list_); }
+  int GetCount() const { return CountSetBits(list_); }
   static RegisterList Union(const RegisterList& list_1,
                             const RegisterList& list_2) {
     return RegisterList(list_1.list_ | list_2.list_);
diff --git a/src/aarch32/macro-assembler-aarch32.h b/src/aarch32/macro-assembler-aarch32.h
index 62c116d5..59b25a5d 100644
--- a/src/aarch32/macro-assembler-aarch32.h
+++ b/src/aarch32/macro-assembler-aarch32.h
@@ -2922,7 +2922,7 @@ class MacroAssembler : public Assembler, public MacroAssemblerInterface {
     VIXL_ASSERT(OutsideITBlock());
     MacroEmissionCheckScope guard(this);
     ITScope it_scope(this, &cond, guard);
-    if (registers.IsSingleRegister() &&
+    if (registers.IsSingleRegister() && !registers.Includes(sp) &&
         (!IsUsingT32() || !registers.IsR0toR7orLR())) {
       push(cond, registers.GetFirstAvailableRegister());
     } else if (!registers.IsEmpty()) {
@@ -2937,7 +2937,12 @@ class MacroAssembler : public Assembler, public MacroAssemblerInterface {
     VIXL_ASSERT(OutsideITBlock());
     MacroEmissionCheckScope guard(this);
     ITScope it_scope(this, &cond, guard);
-    push(cond, rt);
+    if (IsUsingA32() && rt.IsSP()) {
+      // Only the A32 multiple-register form can push sp.
+      push(cond, RegisterList(rt));
+    } else {
+      push(cond, rt);
+    }
   }
   void Push(Register rt) { Push(al, rt); }
 
diff --git a/test/aarch32/test-disasm-a32.cc b/test/aarch32/test-disasm-a32.cc
index cb3a3761..cda3de43 100644
--- a/test/aarch32/test-disasm-a32.cc
+++ b/test/aarch32/test-disasm-a32.cc
@@ -2507,22 +2507,31 @@ TEST(macro_assembler_PushRegisterList) {
               "beq 0x00000006\n"
               "push {r0,r1,r2,r3,r4,r5,r6,r7,r8,r9,r10,r11,ip}\n");
 
-  COMPARE_A32(Push(RegisterList(sp)), "push {sp}\n");
+  // Narrow form, T1.
+  COMPARE_T32(Pop(RegisterList(r0)), "pop {r0}\n");
+  // <single_register_list> form, T4
+  COMPARE_T32(Pop(RegisterList(r10)), "pop {r10}\n");
 
-  // TODO: Clarify behaviour of MacroAssembler vs Assembler with respect to
-  //       deprecated and unpredictable instructions. The tests reflect the
-  //       current behaviour and will need to be updated.
+  // It is usually UNPREDICTABLE to push sp.
+  MUST_FAIL_TEST_BOTH(Push(RegisterList(r0, sp)),
+                      "Unpredictable instruction.\n");
+  MUST_FAIL_TEST_T32(Push(RegisterList(sp)), "Unpredictable instruction.\n");
+  MUST_FAIL_TEST_T32(Push(sp), "Unpredictable instruction.\n");
+  // A32 can push sp if it is the first register in the list.
+  COMPARE_A32(Push(sp), "stmdb sp!, {sp}\n");
+  COMPARE_A32(Push(RegisterList(sp)), "stmdb sp!, {sp}\n");
+  COMPARE_A32(Push(RegisterList(sp, lr)), "push {sp,lr}\n");
 
-  // We don't accept the single-register version of this.
-  MUST_FAIL_TEST_BOTH(Push(pc), "Unpredictable instruction.\n");
-  // The macro assembler translates a single-register register list into a
-  // single-register push.
-  MUST_FAIL_TEST_A32(Push(RegisterList(pc)), "Unpredictable instruction.\n");
   // Deprecated, but accepted:
+  SHOULD_FAIL_TEST_A32(Push(pc));
+  SHOULD_FAIL_TEST_A32(Push(RegisterList(pc)));
   SHOULD_FAIL_TEST_A32(Push(RegisterList(r0, pc)));
 
-  // Accepted, but stores UNKNOWN value for the SP:
-  SHOULD_FAIL_TEST_A32(Push(RegisterList(r0, sp)));
+  MUST_FAIL_TEST_T32(Push(pc), "Unpredictable instruction.\n");
+  MUST_FAIL_TEST_T32(Push(RegisterList(pc)), "Unpredictable instruction.\n");
+  // The multiple-register T32 push can't encode PC at all.
+  MUST_FAIL_TEST_T32(Push(RegisterList(r0, pc)),
+                     "Ill-formed 'push' instruction.\n");
 
   // The following use the PUSH (T1) and PUSH (single register) (A1) encodings
   // for T32 and A32 respectively:
@@ -2533,12 +2542,6 @@ TEST(macro_assembler_PushRegisterList) {
   // PUSH (single register), T4 and A1 encodings:
   COMPARE_BOTH(Push(RegisterList(r8)), "push {r8}\n");
 
-  // Cannot push the sp and pc in T32 when using a register list.
-  MUST_FAIL_TEST_T32(Push(RegisterList(r0, sp)),
-                     "Ill-formed 'push' instruction.\n");
-  MUST_FAIL_TEST_T32(Push(RegisterList(r0, pc)),
-                     "Ill-formed 'push' instruction.\n");
-
   // Pushing zero registers should produce no instructions.
   COMPARE_BOTH(Push(RegisterList()), "");
 
@@ -2567,13 +2570,16 @@ TEST(macro_assembler_PopRegisterList) {
               "beq 0x00000006\n"
               "pop {r0,r1,r2,r3,r4,r5,r6,r7,r8,r9,r10,r11,ip}\n");
 
-  // Popping a single register when using a register list becomes a
-  // single-register pop.
-  COMPARE_T32(Pop(RegisterList(sp)), "pop {sp}\n");
+  // Narrow form, T1.
+  COMPARE_T32(Pop(RegisterList(r0)), "pop {r0}\n");
+  // <single_register_list> form, T4.
+  COMPARE_T32(Pop(RegisterList(r10)), "pop {r10}\n");
 
-  // Cannot pop the sp in T32 when using a register list.
-  MUST_FAIL_TEST_T32(Pop(RegisterList(r0, sp)),
-                     "Ill-formed 'pop' instruction.\n");
+  // It is UNPREDICTABLE to pop sp.
+  MUST_FAIL_TEST_BOTH(Pop(RegisterList(r0, sp)),
+                      "Unpredictable instruction.\n");
+  MUST_FAIL_TEST_BOTH(Pop(RegisterList(sp)), "Unpredictable instruction.\n");
+  MUST_FAIL_TEST_BOTH(Pop(sp), "Unpredictable instruction.\n");
 
   // The following use the POP (T1) and POP (single register) (A1) encodings for
   // T32 and A32 respectively:
@@ -2585,10 +2591,9 @@ TEST(macro_assembler_PopRegisterList) {
   COMPARE_BOTH(Pop(RegisterList(r8)), "pop {r8}\n");
   COMPARE_BOTH(Pop(RegisterList(lr)), "pop {lr}\n");
 
-  // TODO: Pushing both the lr and pc should not be allowed by the
-  //       MacroAssembler (deprecated for A32, for T32 they shouldn't both
-  //       be in the list).
-  SHOULD_FAIL_TEST_BOTH(Pop(RegisterList(lr, pc)));
+  MUST_FAIL_TEST_T32(Pop(RegisterList(lr, pc)), "Unpredictable instruction.\n");
+  // Deprecated, but allowed.
+  COMPARE_A32(Pop(RegisterList(lr, pc)), "pop {lr,pc}\n");
 
   // Popping zero registers should produce no instructions.
   COMPARE_BOTH(Pop(RegisterList()), "");