Revert "Enable store elimination for singleton objects."
This reverts commit 7f43a3d48fc29045875d50e10bbc5d6ffc25d61e.
Fails booting.
Bug: 25357772
Change-Id: Ied19536f3ce8d81e76885cb6baed4853e2ed6714
diff --git a/compiler/dex/quick/gen_common.cc b/compiler/dex/quick/gen_common.cc
index 5da7214..2b60a51 100644
--- a/compiler/dex/quick/gen_common.cc
+++ b/compiler/dex/quick/gen_common.cc
@@ -1104,11 +1104,7 @@
// access because the verifier was unable to?
const DexFile* dex_file = cu_->dex_file;
CompilerDriver* driver = cu_->compiler_driver;
- bool finalizable;
- if (driver->CanAccessInstantiableTypeWithoutChecks(cu_->method_idx,
- *dex_file,
- type_idx,
- &finalizable)) {
+ if (driver->CanAccessInstantiableTypeWithoutChecks(cu_->method_idx, *dex_file, type_idx)) {
bool is_type_initialized;
bool use_direct_type_ptr;
uintptr_t direct_type_ptr;
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index fa3598e..8750aa8 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -1247,8 +1247,7 @@
bool CompilerDriver::CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_idx,
const DexFile& dex_file,
- uint32_t type_idx,
- bool* finalizable) {
+ uint32_t type_idx) {
ScopedObjectAccess soa(Thread::Current());
mirror::DexCache* dex_cache = Runtime::Current()->GetClassLinker()->FindDexCache(
soa.Self(), dex_file, false);
@@ -1256,11 +1255,8 @@
mirror::Class* resolved_class = dex_cache->GetResolvedType(type_idx);
if (resolved_class == nullptr) {
stats_->TypeNeedsAccessCheck();
- // Be conservative.
- *finalizable = true;
return false; // Unknown class needs access checks.
}
- *finalizable = resolved_class->IsFinalizable();
const DexFile::MethodId& method_id = dex_file.GetMethodId(referrer_idx);
mirror::Class* referrer_class = dex_cache->GetResolvedType(method_id.class_idx_);
if (referrer_class == nullptr) {
diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index 15806b5..485cdcf 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -200,10 +200,8 @@
REQUIRES(!Locks::mutator_lock_);
// Are runtime access and instantiable checks necessary in the code?
- bool CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_idx,
- const DexFile& dex_file,
- uint32_t type_idx,
- bool* finalizable)
+ bool CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_idx, const DexFile& dex_file,
+ uint32_t type_idx)
REQUIRES(!Locks::mutator_lock_);
bool CanEmbedTypeInCode(const DexFile& dex_file, uint32_t type_idx,
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index ee5b929..ed193c7 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -1455,8 +1455,7 @@
uint32_t* args,
uint32_t register_index) {
HInstruction* length = graph_->GetIntConstant(number_of_vreg_arguments, dex_pc);
- bool finalizable;
- QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index, &finalizable)
+ QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index)
? kQuickAllocArrayWithAccessCheck
: kQuickAllocArray;
HInstruction* object = new (arena_) HNewArray(length,
@@ -1636,9 +1635,9 @@
}
}
-bool HGraphBuilder::NeedsAccessCheck(uint32_t type_index, bool* finalizable) const {
+bool HGraphBuilder::NeedsAccessCheck(uint32_t type_index) const {
return !compiler_driver_->CanAccessInstantiableTypeWithoutChecks(
- dex_compilation_unit_->GetDexMethodIndex(), *dex_file_, type_index, finalizable);
+ dex_compilation_unit_->GetDexMethodIndex(), *dex_file_, type_index);
}
void HGraphBuilder::BuildSwitchJumpTable(const SwitchTable& table,
@@ -2515,9 +2514,7 @@
current_block_->AddInstruction(fake_string);
UpdateLocal(register_index, fake_string, dex_pc);
} else {
- bool finalizable;
- bool can_throw = NeedsAccessCheck(type_index, &finalizable);
- QuickEntrypointEnum entrypoint = can_throw
+ QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index)
? kQuickAllocObjectWithAccessCheck
: kQuickAllocObject;
@@ -2526,8 +2523,6 @@
dex_pc,
type_index,
*dex_compilation_unit_->GetDexFile(),
- can_throw,
- finalizable,
entrypoint));
UpdateLocal(instruction.VRegA(), current_block_->GetLastInstruction(), dex_pc);
}
@@ -2537,8 +2532,7 @@
case Instruction::NEW_ARRAY: {
uint16_t type_index = instruction.VRegC_22c();
HInstruction* length = LoadLocal(instruction.VRegB_22c(), Primitive::kPrimInt, dex_pc);
- bool finalizable;
- QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index, &finalizable)
+ QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index)
? kQuickAllocArrayWithAccessCheck
: kQuickAllocArray;
current_block_->AddInstruction(new (arena_) HNewArray(length,
diff --git a/compiler/optimizing/builder.h b/compiler/optimizing/builder.h
index 0f64489..9eaa4b6 100644
--- a/compiler/optimizing/builder.h
+++ b/compiler/optimizing/builder.h
@@ -138,7 +138,7 @@
HInstruction* LoadLocal(uint32_t register_index, Primitive::Type type, uint32_t dex_pc) const;
void PotentiallyAddSuspendCheck(HBasicBlock* target, uint32_t dex_pc);
void InitializeParameters(uint16_t number_of_parameters);
- bool NeedsAccessCheck(uint32_t type_index, bool* finalizable) const;
+ bool NeedsAccessCheck(uint32_t type_index) const;
template<typename T>
void Unop_12x(const Instruction& instruction, Primitive::Type type, uint32_t dex_pc);
diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc
index aa9c315..90f28e5 100644
--- a/compiler/optimizing/load_store_elimination.cc
+++ b/compiler/optimizing/load_store_elimination.cc
@@ -695,12 +695,8 @@
} else {
redundant_store = true;
}
- HNewInstance* new_instance = ref_info->GetReference()->AsNewInstance();
- DCHECK(new_instance != nullptr);
- if (new_instance->IsFinalizable()) {
- // Finalizable objects escape globally. Need to keep the store.
- redundant_store = false;
- }
+ // TODO: eliminate the store if the singleton object is not finalizable.
+ redundant_store = false;
}
if (redundant_store) {
removed_instructions_.push_back(instruction);
@@ -838,9 +834,7 @@
return;
}
if (!heap_location_collector_.MayDeoptimize() &&
- ref_info->IsSingletonAndNotReturned() &&
- !new_instance->IsFinalizable() &&
- !new_instance->CanThrow()) {
+ ref_info->IsSingletonAndNotReturned()) {
// The allocation might be eliminated.
singleton_new_instances_.push_back(new_instance);
}
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 7ac39d1..6028d4b 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -3595,14 +3595,10 @@
uint32_t dex_pc,
uint16_t type_index,
const DexFile& dex_file,
- bool can_throw,
- bool finalizable,
QuickEntrypointEnum entrypoint)
: HExpression(Primitive::kPrimNot, SideEffects::CanTriggerGC(), dex_pc),
type_index_(type_index),
dex_file_(dex_file),
- can_throw_(can_throw),
- finalizable_(finalizable),
entrypoint_(entrypoint) {
SetRawInputAt(0, current_method);
}
@@ -3612,11 +3608,11 @@
// Calls runtime so needs an environment.
bool NeedsEnvironment() const OVERRIDE { return true; }
-
- // It may throw when called on type that's not instantiable/accessible.
- bool CanThrow() const OVERRIDE { return can_throw_; }
-
- bool IsFinalizable() const { return finalizable_; }
+ // It may throw when called on:
+ // - interfaces
+ // - abstract/innaccessible/unknown classes
+ // TODO: optimize when possible.
+ bool CanThrow() const OVERRIDE { return true; }
bool CanBeNull() const OVERRIDE { return false; }
@@ -3627,8 +3623,6 @@
private:
const uint16_t type_index_;
const DexFile& dex_file_;
- const bool can_throw_;
- const bool finalizable_;
const QuickEntrypointEnum entrypoint_;
DISALLOW_COPY_AND_ASSIGN(HNewInstance);
diff --git a/test/530-checker-lse/src/Main.java b/test/530-checker-lse/src/Main.java
index 924ff67..c766aaa 100644
--- a/test/530-checker-lse/src/Main.java
+++ b/test/530-checker-lse/src/Main.java
@@ -22,7 +22,7 @@
return radius * radius * Math.PI;
}
private double radius;
-}
+};
class TestClass {
TestClass() {
@@ -36,29 +36,16 @@
volatile int k;
TestClass next;
static int si;
-}
+};
class SubTestClass extends TestClass {
int k;
-}
+};
class TestClass2 {
int i;
int j;
-}
-
-class Finalizable {
- static boolean sVisited = false;
- static final int VALUE = 0xbeef;
- int i;
-
- protected void finalize() {
- if (i != VALUE) {
- System.out.println("Where is the beef?");
- }
- sVisited = true;
- }
-}
+};
public class Main {
@@ -69,7 +56,7 @@
/// CHECK-START: double Main.calcCircleArea(double) load_store_elimination (after)
/// CHECK: NewInstance
- /// CHECK-NOT: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
/// CHECK-NOT: InstanceFieldGet
static double calcCircleArea(double radius) {
@@ -130,7 +117,7 @@
/// CHECK: InstanceFieldGet
/// CHECK: InstanceFieldSet
/// CHECK: NewInstance
- /// CHECK-NOT: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
/// CHECK-NOT: InstanceFieldGet
// A new allocation shouldn't alias with pre-existing values.
@@ -236,7 +223,7 @@
/// CHECK-START: int Main.test8() load_store_elimination (after)
/// CHECK: NewInstance
- /// CHECK-NOT: InstanceFieldSet
+ /// CHECK: InstanceFieldSet
/// CHECK: InvokeVirtual
/// CHECK-NOT: NullCheck
/// CHECK-NOT: InstanceFieldGet
@@ -394,8 +381,8 @@
/// CHECK-START: int Main.test16() load_store_elimination (after)
/// CHECK: NewInstance
- /// CHECK-NOT: InstanceFieldSet
- /// CHECK-NOT: InstanceFieldGet
+ /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: StaticFieldGet
// Test inlined constructor.
static int test16() {
@@ -411,8 +398,8 @@
/// CHECK-START: int Main.test17() load_store_elimination (after)
/// CHECK: <<Const0:i\d+>> IntConstant 0
/// CHECK: NewInstance
- /// CHECK-NOT: InstanceFieldSet
- /// CHECK-NOT: InstanceFieldGet
+ /// CHECK-NOT: StaticFieldSet
+ /// CHECK-NOT: StaticFieldGet
/// CHECK: Return [<<Const0>>]
// Test getting default value.
@@ -468,55 +455,6 @@
return obj;
}
- /// CHECK-START: void Main.testFinalizable() load_store_elimination (before)
- /// CHECK: NewInstance
- /// CHECK: InstanceFieldSet
-
- /// CHECK-START: void Main.testFinalizable() load_store_elimination (after)
- /// CHECK: NewInstance
- /// CHECK: InstanceFieldSet
-
- // Allocations and stores into finalizable objects cannot be eliminated.
- static void testFinalizable() {
- Finalizable finalizable = new Finalizable();
- finalizable.i = Finalizable.VALUE;
- }
-
- static java.lang.ref.WeakReference<Object> getWeakReference() {
- return new java.lang.ref.WeakReference<>(new Object());
- }
-
- static void testFinalizableByForcingGc() {
- testFinalizable();
- java.lang.ref.WeakReference<Object> reference = getWeakReference();
-
- Runtime runtime = Runtime.getRuntime();
- for (int i = 0; i < 20; ++i) {
- runtime.gc();
- System.runFinalization();
- try {
- Thread.sleep(1);
- } catch (InterruptedException e) {
- throw new AssertionError(e);
- }
-
- // Check to see if the weak reference has been garbage collected.
- if (reference.get() == null) {
- // A little bit more sleep time to make sure.
- try {
- Thread.sleep(100);
- } catch (InterruptedException e) {
- throw new AssertionError(e);
- }
- if (!Finalizable.sVisited) {
- System.out.println("finalize() not called.");
- }
- return;
- }
- }
- System.out.println("testFinalizableByForcingGc() failed to force gc.");
- }
-
public static void assertIntEquals(int expected, int result) {
if (expected != result) {
throw new Error("Expected: " + expected + ", found: " + result);
@@ -570,6 +508,5 @@
float[] fa2 = { 1.8f };
assertFloatEquals(test19(fa1, fa2), 1.8f);
assertFloatEquals(test20().i, 0);
- testFinalizableByForcingGc();
}
}
