Remove the no-longer-needed F/I and D/J alias.
Rationale:
Now that our HIR is type clean (yeah!), we no longer have
to conservatively assume F/I and D/J are aliased. This
enables more accurate side effects analysis, with improvements
in all clients, such a LICM.
BUG=22538329
Change-Id: Iba1fb09ff063f31b5893f588aa6d0c5ab3b42f39
diff --git a/compiler/optimizing/licm_test.cc b/compiler/optimizing/licm_test.cc
index d446539..2a62643 100644
--- a/compiler/optimizing/licm_test.cc
+++ b/compiler/optimizing/licm_test.cc
@@ -169,13 +169,11 @@
BuildLoop();
// Populate the loop with instructions: set/get array with different types.
- // ArrayGet is typed as kPrimByte and ArraySet given a float value in order to
- // avoid SsaBuilder's typing of ambiguous array operations from reference type info.
HInstruction* get_array = new (&allocator_) HArrayGet(
- parameter_, int_constant_, Primitive::kPrimByte, 0);
+ parameter_, int_constant_, Primitive::kPrimInt, 0);
loop_body_->InsertInstructionBefore(get_array, loop_body_->GetLastInstruction());
HInstruction* set_array = new (&allocator_) HArraySet(
- parameter_, int_constant_, float_constant_, Primitive::kPrimShort, 0);
+ parameter_, int_constant_, float_constant_, Primitive::kPrimFloat, 0);
loop_body_->InsertInstructionBefore(set_array, loop_body_->GetLastInstruction());
EXPECT_EQ(get_array->GetBlock(), loop_body_);
@@ -189,13 +187,11 @@
BuildLoop();
// Populate the loop with instructions: set/get array with same types.
- // ArrayGet is typed as kPrimByte and ArraySet given a float value in order to
- // avoid SsaBuilder's typing of ambiguous array operations from reference type info.
HInstruction* get_array = new (&allocator_) HArrayGet(
- parameter_, int_constant_, Primitive::kPrimByte, 0);
+ parameter_, int_constant_, Primitive::kPrimFloat, 0);
loop_body_->InsertInstructionBefore(get_array, loop_body_->GetLastInstruction());
HInstruction* set_array = new (&allocator_) HArraySet(
- parameter_, get_array, float_constant_, Primitive::kPrimByte, 0);
+ parameter_, get_array, float_constant_, Primitive::kPrimFloat, 0);
loop_body_->InsertInstructionBefore(set_array, loop_body_->GetLastInstruction());
EXPECT_EQ(get_array->GetBlock(), loop_body_);
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index dc5a8fa..6f3e536 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -1551,21 +1551,21 @@
static SideEffects FieldWriteOfType(Primitive::Type type, bool is_volatile) {
return is_volatile
? AllWritesAndReads()
- : SideEffects(TypeFlagWithAlias(type, kFieldWriteOffset));
+ : SideEffects(TypeFlag(type, kFieldWriteOffset));
}
static SideEffects ArrayWriteOfType(Primitive::Type type) {
- return SideEffects(TypeFlagWithAlias(type, kArrayWriteOffset));
+ return SideEffects(TypeFlag(type, kArrayWriteOffset));
}
static SideEffects FieldReadOfType(Primitive::Type type, bool is_volatile) {
return is_volatile
? AllWritesAndReads()
- : SideEffects(TypeFlagWithAlias(type, kFieldReadOffset));
+ : SideEffects(TypeFlag(type, kFieldReadOffset));
}
static SideEffects ArrayReadOfType(Primitive::Type type) {
- return SideEffects(TypeFlagWithAlias(type, kArrayReadOffset));
+ return SideEffects(TypeFlag(type, kArrayReadOffset));
}
static SideEffects CanTriggerGC() {
@@ -1692,23 +1692,6 @@
static constexpr uint64_t kAllReads =
((1ULL << (kLastBitForReads + 1 - kFieldReadOffset)) - 1) << kFieldReadOffset;
- // Work around the fact that HIR aliases I/F and J/D.
- // TODO: remove this interceptor once HIR types are clean
- static uint64_t TypeFlagWithAlias(Primitive::Type type, int offset) {
- switch (type) {
- case Primitive::kPrimInt:
- case Primitive::kPrimFloat:
- return TypeFlag(Primitive::kPrimInt, offset) |
- TypeFlag(Primitive::kPrimFloat, offset);
- case Primitive::kPrimLong:
- case Primitive::kPrimDouble:
- return TypeFlag(Primitive::kPrimLong, offset) |
- TypeFlag(Primitive::kPrimDouble, offset);
- default:
- return TypeFlag(type, offset);
- }
- }
-
// Translates type to bit flag.
static uint64_t TypeFlag(Primitive::Type type, int offset) {
CHECK_NE(type, Primitive::kPrimVoid);
@@ -5196,10 +5179,8 @@
uint32_t dex_pc,
SideEffects additional_side_effects = SideEffects::None())
: HTemplateInstruction(
- SideEffects::ArrayWriteOfType(expected_component_type).Union(
- SideEffectsForArchRuntimeCalls(value->GetType())).Union(
- additional_side_effects),
- dex_pc) {
+ SideEffectsForArchRuntimeCalls(value->GetType()).Union(additional_side_effects),
+ dex_pc) {
SetPackedField<ExpectedComponentTypeField>(expected_component_type);
SetPackedFlag<kFlagNeedsTypeCheck>(value->GetType() == Primitive::kPrimNot);
SetPackedFlag<kFlagValueCanBeNull>(true);
@@ -5207,6 +5188,8 @@
SetRawInputAt(0, array);
SetRawInputAt(1, index);
SetRawInputAt(2, value);
+ // We can now call component type logic to set correct type-based side effects.
+ AddSideEffects(SideEffects::ArrayWriteOfType(GetComponentType()));
}
bool NeedsEnvironment() const OVERRIDE {
diff --git a/compiler/optimizing/side_effects_test.cc b/compiler/optimizing/side_effects_test.cc
index 9bbc354..b01bc1c 100644
--- a/compiler/optimizing/side_effects_test.cc
+++ b/compiler/optimizing/side_effects_test.cc
@@ -148,19 +148,19 @@
EXPECT_FALSE(any_write.MayDependOn(volatile_read));
}
-TEST(SideEffectsTest, SameWidthTypes) {
+TEST(SideEffectsTest, SameWidthTypesNoAlias) {
// Type I/F.
- testWriteAndReadDependence(
+ testNoWriteAndReadDependence(
SideEffects::FieldWriteOfType(Primitive::kPrimInt, /* is_volatile */ false),
SideEffects::FieldReadOfType(Primitive::kPrimFloat, /* is_volatile */ false));
- testWriteAndReadDependence(
+ testNoWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimInt),
SideEffects::ArrayReadOfType(Primitive::kPrimFloat));
// Type L/D.
- testWriteAndReadDependence(
+ testNoWriteAndReadDependence(
SideEffects::FieldWriteOfType(Primitive::kPrimLong, /* is_volatile */ false),
SideEffects::FieldReadOfType(Primitive::kPrimDouble, /* is_volatile */ false));
- testWriteAndReadDependence(
+ testNoWriteAndReadDependence(
SideEffects::ArrayWriteOfType(Primitive::kPrimLong),
SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
}
@@ -216,14 +216,32 @@
"||||||L|",
SideEffects::FieldWriteOfType(Primitive::kPrimNot, false).ToString().c_str());
EXPECT_STREQ(
+ "||DFJISCBZL|DFJISCBZL||DFJISCBZL|DFJISCBZL|",
+ SideEffects::FieldWriteOfType(Primitive::kPrimNot, true).ToString().c_str());
+ EXPECT_STREQ(
"|||||Z||",
SideEffects::ArrayWriteOfType(Primitive::kPrimBoolean).ToString().c_str());
EXPECT_STREQ(
+ "|||||C||",
+ SideEffects::ArrayWriteOfType(Primitive::kPrimChar).ToString().c_str());
+ EXPECT_STREQ(
+ "|||||S||",
+ SideEffects::ArrayWriteOfType(Primitive::kPrimShort).ToString().c_str());
+ EXPECT_STREQ(
"|||B||||",
SideEffects::FieldReadOfType(Primitive::kPrimByte, false).ToString().c_str());
EXPECT_STREQ(
- "||DJ|||||", // note: DJ alias
+ "||D|||||",
SideEffects::ArrayReadOfType(Primitive::kPrimDouble).ToString().c_str());
+ EXPECT_STREQ(
+ "||J|||||",
+ SideEffects::ArrayReadOfType(Primitive::kPrimLong).ToString().c_str());
+ EXPECT_STREQ(
+ "||F|||||",
+ SideEffects::ArrayReadOfType(Primitive::kPrimFloat).ToString().c_str());
+ EXPECT_STREQ(
+ "||I|||||",
+ SideEffects::ArrayReadOfType(Primitive::kPrimInt).ToString().c_str());
SideEffects s = SideEffects::None();
s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimChar, /* is_volatile */ false));
s = s.Union(SideEffects::FieldWriteOfType(Primitive::kPrimLong, /* is_volatile */ false));
@@ -231,9 +249,7 @@
s = s.Union(SideEffects::FieldReadOfType(Primitive::kPrimInt, /* is_volatile */ false));
s = s.Union(SideEffects::ArrayReadOfType(Primitive::kPrimFloat));
s = s.Union(SideEffects::ArrayReadOfType(Primitive::kPrimDouble));
- EXPECT_STREQ(
- "||DFJI|FI||S|DJC|", // note: DJ/FI alias.
- s.ToString().c_str());
+ EXPECT_STREQ("||DF|I||S|JC|", s.ToString().c_str());
}
} // namespace art
diff --git a/test/525-checker-arrays-and-fields/expected.txt b/test/525-checker-arrays-and-fields/expected.txt
index e69de29..b0aad4d 100644
--- a/test/525-checker-arrays-and-fields/expected.txt
+++ b/test/525-checker-arrays-and-fields/expected.txt
@@ -0,0 +1 @@
+passed
diff --git a/test/525-checker-arrays-and-fields/src/Main.java b/test/525-checker-arrays-and-fields/src/Main.java
index a635a51..b3fbf44 100644
--- a/test/525-checker-arrays-and-fields/src/Main.java
+++ b/test/525-checker-arrays-and-fields/src/Main.java
@@ -759,12 +759,87 @@
}
//
+ // Misc. tests on false cross-over loops with data types (I/F and J/D) that used
+ // to be aliased in an older version of the compiler. This alias has been removed,
+ // however, which enables hoisting the invariant array reference.
+ //
+
+ /// CHECK-START: void Main.FalseCrossOverLoop1() licm (before)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:{{B\d+}}
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ /// CHECK-START: void Main.FalseCrossOverLoop1() licm (after)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:none
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ private static void FalseCrossOverLoop1() {
+ sArrF[20] = -1;
+ for (int i = 0; i < sArrI.length; i++) {
+ sArrI[i] = (int) sArrF[20] - 2;
+ }
+ }
+
+ /// CHECK-START: void Main.FalseCrossOverLoop2() licm (before)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:{{B\d+}}
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ /// CHECK-START: void Main.FalseCrossOverLoop2() licm (after)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:none
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ private static void FalseCrossOverLoop2() {
+ sArrI[20] = -2;
+ for (int i = 0; i < sArrF.length; i++) {
+ sArrF[i] = sArrI[20] - 2;
+ }
+ }
+
+ /// CHECK-START: void Main.FalseCrossOverLoop3() licm (before)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:{{B\d+}}
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ /// CHECK-START: void Main.FalseCrossOverLoop3() licm (after)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:none
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ private static void FalseCrossOverLoop3() {
+ sArrD[20] = -3;
+ for (int i = 0; i < sArrJ.length; i++) {
+ sArrJ[i] = (long) sArrD[20] - 2;
+ }
+ }
+
+ /// CHECK-START: void Main.FalseCrossOverLoop4() licm (before)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:{{B\d+}}
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ /// CHECK-START: void Main.FalseCrossOverLoop4() licm (after)
+ /// CHECK-DAG: ArraySet loop:none
+ /// CHECK-DAG: ArrayGet loop:none
+ /// CHECK-DAG: ArraySet loop:{{B\d+}}
+
+ private static void FalseCrossOverLoop4() {
+ sArrJ[20] = -4;
+ for (int i = 0; i < sArrD.length; i++) {
+ sArrD[i] = sArrJ[20] - 2;
+ }
+ }
+
+ //
// Driver and testers.
//
public static void main(String[] args) {
DoStaticTests();
new Main().DoInstanceTests();
+ System.out.println("passed");
}
private static void DoStaticTests() {
@@ -903,6 +978,23 @@
for (int i = 0; i < sArrL.length; i++) {
expectEquals(i <= 20 ? anObject : anotherObject, sArrL[i]);
}
+ // Misc. tests.
+ FalseCrossOverLoop1();
+ for (int i = 0; i < sArrI.length; i++) {
+ expectEquals(-3, sArrI[i]);
+ }
+ FalseCrossOverLoop2();
+ for (int i = 0; i < sArrF.length; i++) {
+ expectEquals(-4, sArrF[i]);
+ }
+ FalseCrossOverLoop3();
+ for (int i = 0; i < sArrJ.length; i++) {
+ expectEquals(-5, sArrJ[i]);
+ }
+ FalseCrossOverLoop4();
+ for (int i = 0; i < sArrD.length; i++) {
+ expectEquals(-6, sArrD[i]);
+ }
}
private void DoInstanceTests() {
