From 513ea0c203a86e9d81a8630b56cb62704e126cc2 Mon Sep 17 00:00:00 2001
From: Andreas Gampe <agampe@google.com>
Date: Mon, 2 Feb 2015 13:17:52 -0800
Subject: [PATCH] ART: Fix thumb2 utils assembler

It didn't correctly put down the 16b variants, as it was not
checking the ShifterOperand correctly.

Add one simple test. TODO: exhaustive testing.

Change-Id: Ied08da115f8417a3ca92bd9967c6dcdc102e7510
---
 compiler/utils/arm/assembler_thumb2.cc      | 72 ++++++++++++++++++++---------
 compiler/utils/arm/assembler_thumb2_test.cc | 27 ++++++++++-
 2 files changed, 76 insertions(+), 23 deletions(-)

diff --git a/compiler/utils/arm/assembler_thumb2.cc b/compiler/utils/arm/assembler_thumb2.cc
index 3eccd3f9d..580614b94 100644
--- a/compiler/utils/arm/assembler_thumb2.cc
+++ b/compiler/utils/arm/assembler_thumb2.cc
@@ -713,7 +713,7 @@ bool Thumb2Assembler::Is32BitDataProcessing(Condition cond ATTRIBUTE_UNUSED,
   }
 
   bool can_contain_high_register = (opcode == MOV)
-      || ((opcode == ADD) && (rn == rd));
+      || ((opcode == ADD) && (rn == rd) && !set_cc);
 
   if (IsHighRegister(rd) || IsHighRegister(rn)) {
     if (!can_contain_high_register) {
@@ -927,41 +927,69 @@ void Thumb2Assembler::Emit16BitDataProcessing(Condition cond,
     if (so.IsImmediate()) {
       use_immediate = true;
       immediate = so.GetImmediate();
+    } else {
+      // Adjust rn and rd: only two registers will be emitted.
+      switch (opcode) {
+        case AND:
+        case ORR:
+        case EOR:
+        case RSB:
+        case ADC:
+        case SBC:
+        case BIC: {
+          if (rn == rd) {
+            rn = so.GetRegister();
+          } else {
+            CHECK_EQ(rd, so.GetRegister());
+          }
+          break;
+        }
+        case CMP:
+        case CMN: {
+          CHECK_EQ(rd, 0);
+          rd = rn;
+          rn = so.GetRegister();
+          break;
+        }
+        case MVN: {
+          CHECK_EQ(rn, 0);
+          rn = so.GetRegister();
+          break;
+        }
+        default:
+          break;
+      }
     }
 
     switch (opcode) {
       case AND: thumb_opcode = 0U /* 0b0000 */; break;
+      case ORR: thumb_opcode = 12U /* 0b1100 */; break;
       case EOR: thumb_opcode = 1U /* 0b0001 */; break;
-      case SUB: break;
       case RSB: thumb_opcode = 9U /* 0b1001 */; break;
-      case ADD: break;
       case ADC: thumb_opcode = 5U /* 0b0101 */; break;
       case SBC: thumb_opcode = 6U /* 0b0110 */; break;
-      case RSC: break;
-      case TST: thumb_opcode = 8U /* 0b1000 */; rn = so.GetRegister(); break;
-      case TEQ: break;
-      case CMP:
+      case BIC: thumb_opcode = 14U /* 0b1110 */; break;
+      case TST: thumb_opcode = 8U /* 0b1000 */; CHECK(!use_immediate); break;
+      case MVN: thumb_opcode = 15U /* 0b1111 */; CHECK(!use_immediate); break;
+      case CMP: {
         if (use_immediate) {
           // T2 encoding.
-           dp_opcode = 0;
-           opcode_shift = 11;
-           thumb_opcode = 5U /* 0b101 */;
-           rd_shift = 8;
-           rn_shift = 8;
+          dp_opcode = 0;
+          opcode_shift = 11;
+          thumb_opcode = 5U /* 0b101 */;
+          rd_shift = 8;
+          rn_shift = 8;
         } else {
           thumb_opcode = 10U /* 0b1010 */;
-          rd = rn;
-          rn = so.GetRegister();
         }
 
         break;
+      }
       case CMN: {
+        CHECK(!use_immediate);
         thumb_opcode = 11U /* 0b1011 */;
-        rd = rn;
-        rn = so.GetRegister();
         break;
       }
-      case ORR: thumb_opcode = 12U /* 0b1100 */; break;
       case MOV:
         dp_opcode = 0;
         if (use_immediate) {
@@ -984,9 +1012,11 @@ void Thumb2Assembler::Emit16BitDataProcessing(Condition cond,
           }
         }
         break;
-      case BIC: thumb_opcode = 14U /* 0b1110 */; break;
-      case MVN: thumb_opcode = 15U /* 0b1111 */; rn = so.GetRegister(); break;
+
+      case TEQ:
+      case RSC:
       default:
+        LOG(FATAL) << "Invalid thumb1 opcode " << opcode;
         break;
     }
   }
@@ -1009,7 +1039,7 @@ void Thumb2Assembler::Emit16BitDataProcessing(Condition cond,
 // ADD and SUB are complex enough to warrant their own emitter.
 void Thumb2Assembler::Emit16BitAddSub(Condition cond ATTRIBUTE_UNUSED,
                                       Opcode opcode,
-                                      bool set_cc ATTRIBUTE_UNUSED,
+                                      bool set_cc,
                                       Register rn,
                                       Register rd,
                                       const ShifterOperand& so) {
@@ -1031,7 +1061,7 @@ void Thumb2Assembler::Emit16BitAddSub(Condition cond ATTRIBUTE_UNUSED,
     case ADD:
       if (so.IsRegister()) {
         Register rm = so.GetRegister();
-        if (rn == rd) {
+        if (rn == rd && !set_cc) {
           // Can use T2 encoding (allows 4 bit registers)
           dp_opcode = 1U /* 0b01 */;
           opcode_shift = 10;
diff --git a/compiler/utils/arm/assembler_thumb2_test.cc b/compiler/utils/arm/assembler_thumb2_test.cc
index 425ccd7ea..e571e7240 100644
--- a/compiler/utils/arm/assembler_thumb2_test.cc
+++ b/compiler/utils/arm/assembler_thumb2_test.cc
@@ -30,11 +30,15 @@ class AssemblerThumb2Test : public AssemblerTest<arm::Thumb2Assembler,
   }
 
   std::string GetAssemblerParameters() OVERRIDE {
-    return " -mthumb -mfpu=neon";
+    return " -march=armv7-a -mcpu=cortex-a15 -mfpu=neon -mthumb";
+  }
+
+  const char* GetAssemblyHeader() OVERRIDE {
+    return kThumb2AssemblyHeader;
   }
 
   std::string GetDisassembleParameters() OVERRIDE {
-    return " -D -bbinary -marm --no-show-raw-insn";
+    return " -D -bbinary -marm --disassembler-options=force-thumb --no-show-raw-insn";
   }
 
   void SetUpHelpers() OVERRIDE {
@@ -76,6 +80,8 @@ class AssemblerThumb2Test : public AssemblerTest<arm::Thumb2Assembler,
 
  private:
   std::vector<arm::Register*> registers_;
+
+  static constexpr const char* kThumb2AssemblyHeader = ".syntax unified\n.thumb\n";
 };
 
 
@@ -192,4 +198,21 @@ TEST_F(AssemblerThumb2Test, strexd) {
   DriverStr(expected, "strexd");
 }
 
+TEST_F(AssemblerThumb2Test, eor) {
+#define __ GetAssembler()->
+  __ eor(arm::R1, arm::R1, arm::ShifterOperand(arm::R0));
+  __ eor(arm::R1, arm::R0, arm::ShifterOperand(arm::R1));
+  __ eor(arm::R1, arm::R8, arm::ShifterOperand(arm::R0));
+  __ eor(arm::R8, arm::R1, arm::ShifterOperand(arm::R0));
+  __ eor(arm::R1, arm::R0, arm::ShifterOperand(arm::R8));
+
+  const char* expected =
+      "eors r1, r0\n"
+      "eor r1, r0, r1\n"
+      "eor r1, r8, r0\n"
+      "eor r8, r1, r0\n"
+      "eor r1, r0, r8\n";
+  DriverStr(expected, "abs");
+}
+
 }  // namespace art
-- 
2.11.0