Fix broken tests.
Rationale:
(1) volatile field write/read need to apply to all
to comply with Java memory model
(2) clinit only needs only the write
(3) added conservative assumptions to memory barrier
(nothing broke, but this seems better)
Change-Id: I37787ec8f3f2c8d6166a94c57193fa4544ad3372
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index badb2bf..85aa004 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -1213,16 +1213,28 @@
return SideEffects(kAllWrites | kAllReads);
}
- static SideEffects FieldWriteOfType(Primitive::Type type) {
- return SideEffects(TypeFlagWithAlias(type, kFieldWriteOffset));
+ static SideEffects AllWrites() {
+ return SideEffects(kAllWrites);
+ }
+
+ static SideEffects AllReads() {
+ return SideEffects(kAllReads);
+ }
+
+ static SideEffects FieldWriteOfType(Primitive::Type type, bool is_volatile) {
+ return is_volatile
+ ? All()
+ : SideEffects(TypeFlagWithAlias(type, kFieldWriteOffset));
}
static SideEffects ArrayWriteOfType(Primitive::Type type) {
return SideEffects(TypeFlagWithAlias(type, kArrayWriteOffset));
}
- static SideEffects FieldReadOfType(Primitive::Type type) {
- return SideEffects(TypeFlagWithAlias(type, kFieldReadOffset));
+ static SideEffects FieldReadOfType(Primitive::Type type, bool is_volatile) {
+ return is_volatile
+ ? All()
+ : SideEffects(TypeFlagWithAlias(type, kFieldReadOffset));
}
static SideEffects ArrayReadOfType(Primitive::Type type) {
@@ -3593,7 +3605,9 @@
bool is_volatile,
uint32_t field_idx,
const DexFile& dex_file)
- : HExpression(field_type, SideEffects::SideEffects::FieldReadOfType(field_type)),
+ : HExpression(
+ field_type,
+ SideEffects::SideEffects::FieldReadOfType(field_type, is_volatile)),
field_info_(field_offset, field_type, is_volatile, field_idx, dex_file) {
SetRawInputAt(0, value);
}
@@ -3635,7 +3649,8 @@
bool is_volatile,
uint32_t field_idx,
const DexFile& dex_file)
- : HTemplateInstruction(SideEffects::FieldWriteOfType(field_type)),
+ : HTemplateInstruction(
+ SideEffects::FieldWriteOfType(field_type, is_volatile)),
field_info_(field_offset, field_type, is_volatile, field_idx, dex_file),
value_can_be_null_(true) {
SetRawInputAt(0, object);
@@ -4005,7 +4020,7 @@
explicit HClinitCheck(HLoadClass* constant, uint32_t dex_pc)
: HExpression(
Primitive::kPrimNot,
- SideEffects::All()), // assume write/read on all fields/arrays
+ SideEffects::AllWrites()), // assume write on all fields/arrays
dex_pc_(dex_pc) {
SetRawInputAt(0, constant);
}
@@ -4041,7 +4056,9 @@
bool is_volatile,
uint32_t field_idx,
const DexFile& dex_file)
- : HExpression(field_type, SideEffects::SideEffects::FieldReadOfType(field_type)),
+ : HExpression(
+ field_type,
+ SideEffects::SideEffects::FieldReadOfType(field_type, is_volatile)),
field_info_(field_offset, field_type, is_volatile, field_idx, dex_file) {
SetRawInputAt(0, cls);
}
@@ -4080,7 +4097,8 @@
bool is_volatile,
uint32_t field_idx,
const DexFile& dex_file)
- : HTemplateInstruction(SideEffects::FieldWriteOfType(field_type)),
+ : HTemplateInstruction(
+ SideEffects::FieldWriteOfType(field_type, is_volatile)),
field_info_(field_offset, field_type, is_volatile, field_idx, dex_file),
value_can_be_null_(true) {
SetRawInputAt(0, cls);
@@ -4255,7 +4273,8 @@
class HMemoryBarrier : public HTemplateInstruction<0> {
public:
explicit HMemoryBarrier(MemBarrierKind barrier_kind)
- : HTemplateInstruction(SideEffects::None()),
+ : HTemplateInstruction(
+ SideEffects::All()), // assume write/read on all fields/arrays
barrier_kind_(barrier_kind) {}
MemBarrierKind GetBarrierKind() { return barrier_kind_; }
diff --git a/compiler/optimizing/side_effects_test.cc b/compiler/optimizing/side_effects_test.cc
index 813f7ce..8db5a8a 100644
--- a/compiler/optimizing/side_effects_test.cc
+++ b/compiler/optimizing/side_effects_test.cc
@@ -95,50 +95,71 @@
type = Primitive::Type(type + 1)) {
// Same primitive type and access type: proper write/read dep.
testWriteAndReadDependence(
- SideEffects::FieldWriteOfType(type),
- SideEffects::FieldReadOfType(type));
+ SideEffects::FieldWriteOfType(type, false),
+ SideEffects::FieldReadOfType(type, false));
testWriteAndReadDependence(
SideEffects::ArrayWriteOfType(type),
SideEffects::ArrayReadOfType(type));
// Same primitive type but different access type: no write/read dep.
testNoWriteAndReadDependence(
- SideEffects::FieldWriteOfType(type),
+ SideEffects::FieldWriteOfType(type, false),
SideEffects::ArrayReadOfType(type));
testNoWriteAndReadDependence(
SideEffects::ArrayWriteOfType(type),
- SideEffects::FieldReadOfType(type));
+ SideEffects::FieldReadOfType(type, false));
}
}
TEST(SideEffectsTest, NoDependences) {
// Different primitive type, same access type: no write/read dep.
testNoWriteAndReadDependence(
- SideEffects::FieldWriteOfType(Primitive::kPrimInt),
- SideEffects::FieldReadOfType(Primitive::kPrimDouble));
+ SideEffects::FieldWriteOfType(Primitive::kPrimInt, false),
+ SideEffects::FieldReadOfType(Primitive::kPrimDouble, false));
testNoWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimInt),
SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
// Everything different: no write/read dep.
testNoWriteAndReadDependence(
- SideEffects::FieldWriteOfType(Primitive::kPrimInt),
+ SideEffects::FieldWriteOfType(Primitive::kPrimInt, false),
SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
testNoWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimInt),
- SideEffects::FieldReadOfType(Primitive::kPrimDouble));
+ SideEffects::FieldReadOfType(Primitive::kPrimDouble, false));
+}
+
+TEST(SideEffectsTest, VolatileDependences) {
+ SideEffects volatile_write =
+ SideEffects::FieldWriteOfType(Primitive::kPrimInt, true);
+ SideEffects any_write =
+ SideEffects::FieldWriteOfType(Primitive::kPrimInt, false);
+ SideEffects volatile_read =
+ SideEffects::FieldReadOfType(Primitive::kPrimByte, true);
+ SideEffects any_read =
+ SideEffects::FieldReadOfType(Primitive::kPrimByte, false);
+
+ EXPECT_FALSE(volatile_write.MayDependOn(any_read));
+ EXPECT_TRUE(any_read.MayDependOn(volatile_write));
+ EXPECT_TRUE(volatile_write.MayDependOn(any_write));
+ EXPECT_FALSE(any_write.MayDependOn(volatile_write));
+
+ EXPECT_FALSE(volatile_read.MayDependOn(any_read));
+ EXPECT_TRUE(any_read.MayDependOn(volatile_read));
+ EXPECT_TRUE(volatile_read.MayDependOn(any_write));
+ EXPECT_FALSE(any_write.MayDependOn(volatile_read));
}
TEST(SideEffectsTest, SameWidthTypes) {
// Type I/F.
testWriteAndReadDependence(
- SideEffects::FieldWriteOfType(Primitive::kPrimInt),
- SideEffects::FieldReadOfType(Primitive::kPrimFloat));
+ SideEffects::FieldWriteOfType(Primitive::kPrimInt, false),
+ SideEffects::FieldReadOfType(Primitive::kPrimFloat, false));
testWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimInt),
SideEffects::ArrayReadOfType(Primitive::kPrimFloat));
// Type L/D.
testWriteAndReadDependence(
- SideEffects::FieldWriteOfType(Primitive::kPrimLong),
- SideEffects::FieldReadOfType(Primitive::kPrimDouble));
+ SideEffects::FieldWriteOfType(Primitive::kPrimLong, false),
+ SideEffects::FieldReadOfType(Primitive::kPrimDouble, false));
testWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimLong),
SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
@@ -150,9 +171,9 @@
for (Primitive::Type type = Primitive::kPrimNot;
type < Primitive::kPrimVoid;
type = Primitive::Type(type + 1)) {
- s = s.Union(SideEffects::FieldWriteOfType(type));
+ s = s.Union(SideEffects::FieldWriteOfType(type, false));
s = s.Union(SideEffects::ArrayWriteOfType(type));
- s = s.Union(SideEffects::FieldReadOfType(type));
+ s = s.Union(SideEffects::FieldReadOfType(type, false));
s = s.Union(SideEffects::ArrayReadOfType(type));
}
EXPECT_TRUE(s.DoesAll());
@@ -166,22 +187,28 @@
"|DFJISCBZL|DFJISCBZL|DFJISCBZL|DFJISCBZL|",
SideEffects::All().ToString().c_str());
EXPECT_STREQ(
+ "|||DFJISCBZL|DFJISCBZL|",
+ SideEffects::AllWrites().ToString().c_str());
+ EXPECT_STREQ(
+ "|DFJISCBZL|DFJISCBZL|||",
+ SideEffects::AllReads().ToString().c_str());
+ EXPECT_STREQ(
"||||L|",
- SideEffects::FieldWriteOfType(Primitive::kPrimNot).ToString().c_str());
+ SideEffects::FieldWriteOfType(Primitive::kPrimNot, false).ToString().c_str());
EXPECT_STREQ(
"|||Z||",
SideEffects::ArrayWriteOfType(Primitive::kPrimBoolean).ToString().c_str());
EXPECT_STREQ(
"||B|||",
- SideEffects::FieldReadOfType(Primitive::kPrimByte).ToString().c_str());
+ SideEffects::FieldReadOfType(Primitive::kPrimByte, false).ToString().c_str());
EXPECT_STREQ(
"|DJ||||", // note: DJ alias
SideEffects::ArrayReadOfType(Primitive::kPrimDouble).ToString().c_str());
SideEffects s = SideEffects::None();
- s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimChar));
- s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimLong));
+ s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimChar, false));
+ s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimLong, false));
s = s.Union(SideEffects::ArrayWriteOfType(Primitive::kPrimShort));
- s = s.Union(SideEffects::FieldReadOfType(Primitive::kPrimInt));
+ s = s.Union(SideEffects::FieldReadOfType(Primitive::kPrimInt, false));
s = s.Union(SideEffects::ArrayReadOfType(Primitive::kPrimFloat));
s = s.Union(SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
EXPECT_STREQ(
