Quick: Fix LVN/GVN handling of acquire operations.
Acquire operations, i.e. MONITOR_ENTER and volatile GETs,
change the thread's view of the memory, so subsequent loads
must get new value names in LVN/GVN. Release operations do
not affect this thread's view of the memory, they the only
push the modifications for other threads to see.
Bug: 17689750
Change-Id: I9442d89b1d2c5252b99b02851b71bb85f871d734
diff --git a/compiler/dex/local_value_numbering_test.cc b/compiler/dex/local_value_numbering_test.cc
index e53c640..067bea2 100644
--- a/compiler/dex/local_value_numbering_test.cc
+++ b/compiler/dex/local_value_numbering_test.cc
@@ -338,16 +338,19 @@
DEF_IGET(Instruction::IGET, 1u, 0u, 0u), // Non-volatile.
DEF_IGET(Instruction::IGET, 2u, 10u, 1u), // Volatile.
DEF_IGET(Instruction::IGET, 3u, 2u, 1u), // Non-volatile.
+ DEF_IGET(Instruction::IGET, 4u, 0u, 0u), // Non-volatile.
};
PrepareIFields(ifields);
PrepareMIRs(mirs);
PerformLVN();
- ASSERT_EQ(value_names_.size(), 4u);
+ ASSERT_EQ(value_names_.size(), 5u);
EXPECT_NE(value_names_[0], value_names_[2]); // Volatile has always different value name.
EXPECT_NE(value_names_[1], value_names_[3]); // Used different base because of volatile.
+ EXPECT_NE(value_names_[1], value_names_[4]); // Not guaranteed to be the same after "acquire".
+
for (size_t i = 0; i != arraysize(mirs); ++i) {
- EXPECT_EQ((i == 2u) ? MIR_IGNORE_NULL_CHECK : 0,
+ EXPECT_EQ((i == 2u || i == 4u) ? MIR_IGNORE_NULL_CHECK : 0,
mirs_[i].optimization_flags) << i;
}
}
@@ -363,7 +366,7 @@
DEF_IGET(Instruction::IGET, 1u, 20u, 0u), // Resolved field #1, unique object.
DEF_IGET(Instruction::IGET, 2u, 21u, 0u), // Resolved field #1.
DEF_IGET_WIDE(Instruction::IGET_WIDE, 3u, 21u, 1u), // Resolved field #2.
- DEF_IGET(Instruction::IGET, 4u, 22u, 2u), // IGET doesn't clobber anything.
+ DEF_IGET(Instruction::IGET, 4u, 22u, 2u), // Unresolved IGET can be "acquire".
DEF_IGET(Instruction::IGET, 5u, 20u, 0u), // Resolved field #1, unique object.
DEF_IGET(Instruction::IGET, 6u, 21u, 0u), // Resolved field #1.
DEF_IGET_WIDE(Instruction::IGET_WIDE, 7u, 21u, 1u), // Resolved field #2.
@@ -381,14 +384,15 @@
PrepareMIRs(mirs);
PerformLVN();
ASSERT_EQ(value_names_.size(), 16u);
- EXPECT_EQ(value_names_[1], value_names_[5]);
- EXPECT_EQ(value_names_[2], value_names_[6]);
- EXPECT_EQ(value_names_[3], value_names_[7]);
- EXPECT_EQ(value_names_[1], value_names_[9]);
- EXPECT_NE(value_names_[2], value_names_[10]); // This aliased with unresolved IPUT.
- EXPECT_EQ(value_names_[3], value_names_[11]);
- EXPECT_EQ(value_names_[12], value_names_[15]);
- EXPECT_NE(value_names_[1], value_names_[14]); // This aliased with unresolved IPUT.
+ // Unresolved field is potentially volatile, so we need to adhere to the volatile semantics.
+ EXPECT_EQ(value_names_[1], value_names_[5]); // Unique object.
+ EXPECT_NE(value_names_[2], value_names_[6]); // Not guaranteed to be the same after "acquire".
+ EXPECT_NE(value_names_[3], value_names_[7]); // Not guaranteed to be the same after "acquire".
+ EXPECT_EQ(value_names_[1], value_names_[9]); // Unique object.
+ EXPECT_NE(value_names_[6], value_names_[10]); // This aliased with unresolved IPUT.
+ EXPECT_EQ(value_names_[7], value_names_[11]); // Still the same after "release".
+ EXPECT_EQ(value_names_[12], value_names_[15]); // Still the same after "release".
+ EXPECT_NE(value_names_[1], value_names_[14]); // This aliased with unresolved IPUT.
EXPECT_EQ(mirs_[0].optimization_flags, 0u);
EXPECT_EQ(mirs_[1].optimization_flags, MIR_IGNORE_NULL_CHECK);
EXPECT_EQ(mirs_[2].optimization_flags, 0u);
@@ -409,7 +413,7 @@
static const MIRDef mirs[] = {
DEF_SGET(Instruction::SGET, 0u, 0u), // Resolved field #1.
DEF_SGET_WIDE(Instruction::SGET_WIDE, 1u, 1u), // Resolved field #2.
- DEF_SGET(Instruction::SGET, 2u, 2u), // SGET doesn't clobber anything.
+ DEF_SGET(Instruction::SGET, 2u, 2u), // Unresolved SGET can be "acquire".
DEF_SGET(Instruction::SGET, 3u, 0u), // Resolved field #1.
DEF_SGET_WIDE(Instruction::SGET_WIDE, 4u, 1u), // Resolved field #2.
DEF_SPUT(Instruction::SPUT, 5u, 2u), // SPUT clobbers field #1 (#2 is wide).
@@ -421,10 +425,11 @@
PrepareMIRs(mirs);
PerformLVN();
ASSERT_EQ(value_names_.size(), 8u);
- EXPECT_EQ(value_names_[0], value_names_[3]);
- EXPECT_EQ(value_names_[1], value_names_[4]);
- EXPECT_NE(value_names_[0], value_names_[6]); // This aliased with unresolved IPUT.
- EXPECT_EQ(value_names_[1], value_names_[7]);
+ // Unresolved field is potentially volatile, so we need to adhere to the volatile semantics.
+ EXPECT_NE(value_names_[0], value_names_[3]); // Not guaranteed to be the same after "acquire".
+ EXPECT_NE(value_names_[1], value_names_[4]); // Not guaranteed to be the same after "acquire".
+ EXPECT_NE(value_names_[3], value_names_[6]); // This aliased with unresolved IPUT.
+ EXPECT_EQ(value_names_[4], value_names_[7]); // Still the same after "release".
for (size_t i = 0u; i != mir_count_; ++i) {
EXPECT_EQ(0, mirs_[i].optimization_flags) << i;
}