From 3b0ee6f06b6643e2a601283e779ba5d574fe1fda Mon Sep 17 00:00:00 2001
From: Vladimir Marko <vmarko@google.com>
Date: Thu, 14 May 2015 11:47:39 +0100
Subject: [PATCH] Quick: Fix marking high words in DCE.

This CL properly fixes the high word marking and reverts
    https://android-review.googlesource.com/150352
which was just covering up the underlying issue. A unit test
for the encountered issue is provided, though it does not
expose the deficiency in the cover-up CL.

Bug: 20640451

(cherry picked from commit 9cacef6e811940c2f21e7e54055379a2c43f0d06)

Change-Id: I4c4ca82fe4b2e34feb38090d88a5d5c754914f89
---
 compiler/dex/gvn_dead_code_elimination.cc      |  9 ++---
 compiler/dex/gvn_dead_code_elimination_test.cc | 52 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/compiler/dex/gvn_dead_code_elimination.cc b/compiler/dex/gvn_dead_code_elimination.cc
index 915fbcda0..6d8a7dab2 100644
--- a/compiler/dex/gvn_dead_code_elimination.cc
+++ b/compiler/dex/gvn_dead_code_elimination.cc
@@ -58,14 +58,12 @@ inline void GvnDeadCodeElimination::MIRData::RemovePrevChange(int v_reg, MIRData
       low_def_over_high_word = prev_data->low_def_over_high_word;
     } else {
       prev_value = prev_data->prev_value_high;
-      low_def_over_high_word =
-          prev_data->prev_value_high.value != kNPos && !prev_data->high_def_over_low_word;
+      low_def_over_high_word = !prev_data->high_def_over_low_word;
     }
   } else {
     if (prev_data->vreg_def == v_reg) {
       prev_value_high = prev_data->prev_value;
-      high_def_over_low_word =
-          prev_data->prev_value.value != kNPos && !prev_data->low_def_over_high_word;
+      high_def_over_low_word = !prev_data->low_def_over_high_word;
     } else {
       prev_value_high = prev_data->prev_value_high;
       high_def_over_low_word = prev_data->high_def_over_low_word;
@@ -340,8 +338,7 @@ void GvnDeadCodeElimination::VRegChains::RemoveChange(uint16_t change) {
       DCHECK_EQ(vreg_high_words_.IsBitSet(v_reg), v_reg == data->vreg_def + 1);
       if (data->vreg_def == v_reg && data->low_def_over_high_word) {
         vreg_high_words_.SetBit(v_reg);
-      } else if (data->vreg_def != v_reg &&
-          (data->high_def_over_low_word || data->prev_value_high.value == kNoValue)) {
+      } else if (data->vreg_def != v_reg && data->high_def_over_low_word) {
         vreg_high_words_.ClearBit(v_reg);
       }
     } else {
diff --git a/compiler/dex/gvn_dead_code_elimination_test.cc b/compiler/dex/gvn_dead_code_elimination_test.cc
index 4f8127338..de591d0ed 100644
--- a/compiler/dex/gvn_dead_code_elimination_test.cc
+++ b/compiler/dex/gvn_dead_code_elimination_test.cc
@@ -1931,4 +1931,56 @@ TEST_F(GvnDeadCodeEliminationTestDiamond, LongOverlaps1) {
   }
 }
 
+TEST_F(GvnDeadCodeEliminationTestSimple, MixedOverlaps1) {
+  static const MIRDef mirs[] = {
+      DEF_CONST(3, Instruction::CONST, 0u, 1000u),
+      DEF_MOVE(3, Instruction::MOVE, 1u, 0u),
+      DEF_CONST(3, Instruction::CONST, 2u, 2000u),
+      { 3, Instruction::INT_TO_LONG, 0, 0u, 1, { 2u }, 2, { 3u, 4u} },
+      DEF_MOVE_WIDE(3, Instruction::MOVE_WIDE, 5u, 3u),
+      DEF_CONST(3, Instruction::CONST, 7u, 3000u),
+      DEF_CONST(3, Instruction::CONST, 8u, 4000u),
+  };
+
+  static const int32_t sreg_to_vreg_map[] = { 1, 2, 0, 0, 1, 3, 4, 0, 1 };
+  PrepareSRegToVRegMap(sreg_to_vreg_map);
+
+  PrepareMIRs(mirs);
+  static const int32_t wide_sregs[] = { 3, 5 };
+  MarkAsWideSRegs(wide_sregs);
+  PerformGVN_DCE();
+
+  ASSERT_EQ(arraysize(mirs), value_names_.size());
+  static const size_t diff_indexes[] = { 0, 2, 3, 5, 6 };
+  ExpectValueNamesNE(diff_indexes);
+  EXPECT_EQ(value_names_[0], value_names_[1]);
+  EXPECT_EQ(value_names_[3], value_names_[4]);
+
+  static const bool eliminated[] = {
+      false, true, false, false, true, false, false,
+  };
+  static_assert(arraysize(eliminated) == arraysize(mirs), "array size mismatch");
+  for (size_t i = 0; i != arraysize(eliminated); ++i) {
+    bool actually_eliminated = (static_cast<int>(mirs_[i].dalvikInsn.opcode) == kMirOpNop);
+    EXPECT_EQ(eliminated[i], actually_eliminated) << i;
+  }
+  // Check renamed registers in CONST.
+  MIR* cst = &mirs_[0];
+  ASSERT_EQ(Instruction::CONST, cst->dalvikInsn.opcode);
+  ASSERT_EQ(0, cst->ssa_rep->num_uses);
+  ASSERT_EQ(1, cst->ssa_rep->num_defs);
+  EXPECT_EQ(1, cst->ssa_rep->defs[0]);
+  EXPECT_EQ(2u, cst->dalvikInsn.vA);
+  // Check renamed registers in INT_TO_LONG.
+  MIR* int_to_long = &mirs_[3];
+  ASSERT_EQ(Instruction::INT_TO_LONG, int_to_long->dalvikInsn.opcode);
+  ASSERT_EQ(1, int_to_long->ssa_rep->num_uses);
+  EXPECT_EQ(2, int_to_long->ssa_rep->uses[0]);
+  ASSERT_EQ(2, int_to_long->ssa_rep->num_defs);
+  EXPECT_EQ(5, int_to_long->ssa_rep->defs[0]);
+  EXPECT_EQ(6, int_to_long->ssa_rep->defs[1]);
+  EXPECT_EQ(3u, int_to_long->dalvikInsn.vA);
+  EXPECT_EQ(0u, int_to_long->dalvikInsn.vB);
+}
+
 }  // namespace art
-- 
2.11.0