[turbofan] Negated immediates for ARM64 add/sub

src/compiler/arm64/instruction-selector-arm64-unittest.cc

Index: src/compiler/arm64/instruction-selector-arm64-unittest.cc
diff --git a/src/compiler/arm64/instruction-selector-arm64-unittest.cc b/src/compiler/arm64/instruction-selector-arm64-unittest.cc
index bd1471156e0d5803d0be1821e5af4a6734511254..6255885203fab8567ce57152eadfc53de4117760 100644
--- a/src/compiler/arm64/instruction-selector-arm64-unittest.cc
+++ b/src/compiler/arm64/instruction-selector-arm64-unittest.cc
@@ -86,11 +86,26 @@ static const uint32_t kLogicalImmediates[] = {
 
 
 // ARM64 arithmetic instructions.
-static const MachInst2 kAddSubInstructions[] = {
-    {&RawMachineAssembler::Int32Add, "Int32Add", kArm64Add32, kMachInt32},
-    {&RawMachineAssembler::Int64Add, "Int64Add", kArm64Add, kMachInt64},
-    {&RawMachineAssembler::Int32Sub, "Int32Sub", kArm64Sub32, kMachInt32},
-    {&RawMachineAssembler::Int64Sub, "Int64Sub", kArm64Sub, kMachInt64}};
+struct AddSub {
+  MachInst2 mi;
+  ArchOpcode negate_arch_opcode;
+};
+
+
+std::ostream& operator<<(std::ostream& os, const AddSub& op) {
+  return os << op.mi;
+}
+
+
+static const AddSub kAddSubInstructions[] = {
+    {{&RawMachineAssembler::Int32Add, "Int32Add", kArm64Add32, kMachInt32},
+     kArm64Sub32},
+    {{&RawMachineAssembler::Int64Add, "Int64Add", kArm64Add, kMachInt64},
+     kArm64Sub},
+    {{&RawMachineAssembler::Int32Sub, "Int32Sub", kArm64Sub32, kMachInt32},
+     kArm64Add32},
+    {{&RawMachineAssembler::Int64Sub, "Int64Sub", kArm64Sub, kMachInt64},
+     kArm64Add}};
 
 
 // ARM64 Add/Sub immediates: 12-bit immediate optionally shifted by 12.
@@ -290,32 +305,32 @@ INSTANTIATE_TEST_CASE_P(InstructionSelectorTest, InstructionSelectorLogicalTest,
 // -----------------------------------------------------------------------------
 // Add and Sub instructions.
 
-typedef InstructionSelectorTestWithParam<MachInst2>
-    InstructionSelectorAddSubTest;
+typedef InstructionSelectorTestWithParam<AddSub> InstructionSelectorAddSubTest;
 
 
 TEST_P(InstructionSelectorAddSubTest, Parameter) {
-  const MachInst2 dpi = GetParam();
-  const MachineType type = dpi.machine_type;
+  const AddSub dpi = GetParam();
+  const MachineType type = dpi.mi.machine_type;
   StreamBuilder m(this, type, type, type);
-  m.Return((m.*dpi.constructor)(m.Parameter(0), m.Parameter(1)));
+  m.Return((m.*dpi.mi.constructor)(m.Parameter(0), m.Parameter(1)));
   Stream s = m.Build();
   ASSERT_EQ(1U, s.size());
-  EXPECT_EQ(dpi.arch_opcode, s[0]->arch_opcode());
+  EXPECT_EQ(dpi.mi.arch_opcode, s[0]->arch_opcode());
   EXPECT_EQ(2U, s[0]->InputCount());
   EXPECT_EQ(1U, s[0]->OutputCount());
 }
 
 
 TEST_P(InstructionSelectorAddSubTest, ImmediateOnRight) {
-  const MachInst2 dpi = GetParam();
-  const MachineType type = dpi.machine_type;
+  const AddSub dpi = GetParam();
+  const MachineType type = dpi.mi.machine_type;
   TRACED_FOREACH(int32_t, imm, kAddSubImmediates) {
     StreamBuilder m(this, type, type);
-    m.Return((m.*dpi.constructor)(m.Parameter(0), BuildConstant(m, type, imm)));
+    m.Return(
+        (m.*dpi.mi.constructor)(m.Parameter(0), BuildConstant(m, type, imm)));
     Stream s = m.Build();
     ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(dpi.arch_opcode, s[0]->arch_opcode());
+    EXPECT_EQ(dpi.mi.arch_opcode, s[0]->arch_opcode());
     ASSERT_EQ(2U, s[0]->InputCount());
     EXPECT_TRUE(s[0]->InputAt(1)->IsImmediate());
     EXPECT_EQ(imm, s.ToInt64(s[0]->InputAt(1)));
@@ -324,20 +339,40 @@ TEST_P(InstructionSelectorAddSubTest, ImmediateOnRight) {
 }
 
 
+TEST_P(InstructionSelectorAddSubTest, NegImmediateOnRight) {
+  const AddSub dpi = GetParam();
+  const MachineType type = dpi.mi.machine_type;
+  TRACED_FOREACH(int32_t, imm, kAddSubImmediates) {
+    if (imm == 0) continue;
+    StreamBuilder m(this, type, type);
+    m.Return(
+        (m.*dpi.mi.constructor)(m.Parameter(0), BuildConstant(m, type, -imm)));
+    Stream s = m.Build();
+    ASSERT_EQ(1U, s.size());
+    EXPECT_EQ(dpi.negate_arch_opcode, s[0]->arch_opcode());
+    ASSERT_EQ(2U, s[0]->InputCount());
+    ASSERT_TRUE(s[0]->InputAt(1)->IsImmediate());
+    EXPECT_EQ(imm, s.ToInt32(s[0]->InputAt(1)));
+    EXPECT_EQ(1U, s[0]->OutputCount());
+  }
+}
+
+
 TEST_P(InstructionSelectorAddSubTest, ImmediateOnLeft) {
-  const MachInst2 dpi = GetParam();
-  const MachineType type = dpi.machine_type;
+  const AddSub dpi = GetParam();
+  const MachineType type = dpi.mi.machine_type;
 
   TRACED_FOREACH(int32_t, imm, kAddSubImmediates) {
     StreamBuilder m(this, type, type);
-    m.Return((m.*dpi.constructor)(BuildConstant(m, type, imm), m.Parameter(0)));
+    m.Return(
+        (m.*dpi.mi.constructor)(BuildConstant(m, type, imm), m.Parameter(0)));
     Stream s = m.Build();
 
     // Add can support an immediate on the left by commuting, but Sub can't
     // commute. We test zero-on-left Sub later.
-    if (strstr(dpi.constructor_name, "Add") != NULL) {
+    if (strstr(dpi.mi.constructor_name, "Add") != NULL) {

[Benedikt Meurer, 2014/09/30 04:47:49]

See comment below.

       ASSERT_EQ(1U, s.size());
-      EXPECT_EQ(dpi.arch_opcode, s[0]->arch_opcode());
+      EXPECT_EQ(dpi.mi.arch_opcode, s[0]->arch_opcode());
       ASSERT_EQ(2U, s[0]->InputCount());
       EXPECT_TRUE(s[0]->InputAt(1)->IsImmediate());
       EXPECT_EQ(imm, s.ToInt64(s[0]->InputAt(1)));
@@ -347,6 +382,31 @@ TEST_P(InstructionSelectorAddSubTest, ImmediateOnLeft) {
 }
 
 
+TEST_P(InstructionSelectorAddSubTest, NegImmediateOnLeft) {
+  const AddSub dpi = GetParam();
+  const MachineType type = dpi.mi.machine_type;
+
+  TRACED_FOREACH(int32_t, imm, kAddSubImmediates) {
+    if (imm == 0) continue;
+    StreamBuilder m(this, type, type);
+    m.Return(
+        (m.*dpi.mi.constructor)(BuildConstant(m, type, -imm), m.Parameter(0)));
+    Stream s = m.Build();
+
+    // Add can support an immediate on the left by commuting, but Sub can't
+    // commute.
+    if (strstr(dpi.mi.constructor_name, "Add") != NULL) {

[Benedikt Meurer, 2014/09/30 04:47:49]

Please split the test case instead of filtering, i.e. add new tests that run
only for add instructions like InstructionSelectorAddTest.

[m.m.capewell, 2014/09/30 17:42:09]

I can do this, or add a new "commutes" bool in the AddSub structure, which would
avoid some duplication. Which would you prefer?

[Benedikt Meurer, 2014/10/01 15:21:46]

Split it, please.

[m.m.capewell, 2014/10/02 14:43:43]

Done.

+      ASSERT_EQ(1U, s.size());
+      EXPECT_EQ(dpi.negate_arch_opcode, s[0]->arch_opcode());
+      ASSERT_EQ(2U, s[0]->InputCount());
+      ASSERT_TRUE(s[0]->InputAt(1)->IsImmediate());
+      EXPECT_EQ(imm, s.ToInt32(s[0]->InputAt(1)));
+      EXPECT_EQ(1U, s[0]->OutputCount());
+    }
+  }
+}
+
+
 INSTANTIATE_TEST_CASE_P(InstructionSelectorTest, InstructionSelectorAddSubTest,
                         ::testing::ValuesIn(kAddSubInstructions));