diff options
-rw-r--r-- | compiler/dex/quick/gen_common.cc | 6 | ||||
-rw-r--r-- | compiler/driver/compiler_driver.cc | 6 | ||||
-rw-r--r-- | compiler/driver/compiler_driver.h | 6 | ||||
-rw-r--r-- | compiler/optimizing/builder.cc | 16 | ||||
-rw-r--r-- | compiler/optimizing/builder.h | 2 | ||||
-rw-r--r-- | compiler/optimizing/load_store_elimination.cc | 12 | ||||
-rw-r--r-- | compiler/optimizing/nodes.h | 16 | ||||
-rw-r--r-- | test/530-checker-lse/src/Main.java | 85 |
8 files changed, 120 insertions, 29 deletions
diff --git a/compiler/dex/quick/gen_common.cc b/compiler/dex/quick/gen_common.cc index 2b60a51e22..5da72147b0 100644 --- a/compiler/dex/quick/gen_common.cc +++ b/compiler/dex/quick/gen_common.cc @@ -1104,7 +1104,11 @@ void Mir2Lir::GenNewInstance(uint32_t type_idx, RegLocation rl_dest) { // access because the verifier was unable to? const DexFile* dex_file = cu_->dex_file; CompilerDriver* driver = cu_->compiler_driver; - if (driver->CanAccessInstantiableTypeWithoutChecks(cu_->method_idx, *dex_file, type_idx)) { + bool finalizable; + if (driver->CanAccessInstantiableTypeWithoutChecks(cu_->method_idx, + *dex_file, + type_idx, + &finalizable)) { 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 8750aa8e4e..fa3598e291 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -1247,7 +1247,8 @@ bool CompilerDriver::CanAccessTypeWithoutChecks(uint32_t referrer_idx, const Dex bool CompilerDriver::CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_idx, const DexFile& dex_file, - uint32_t type_idx) { + uint32_t type_idx, + bool* finalizable) { ScopedObjectAccess soa(Thread::Current()); mirror::DexCache* dex_cache = Runtime::Current()->GetClassLinker()->FindDexCache( soa.Self(), dex_file, false); @@ -1255,8 +1256,11 @@ bool CompilerDriver::CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_id 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 485cdcfb1b..15806b5579 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -200,8 +200,10 @@ class CompilerDriver { 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 CanAccessInstantiableTypeWithoutChecks(uint32_t referrer_idx, + const DexFile& dex_file, + uint32_t type_idx, + bool* finalizable) 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 ed193c7b61..ee5b929875 100644 --- a/compiler/optimizing/builder.cc +++ b/compiler/optimizing/builder.cc @@ -1455,7 +1455,8 @@ void HGraphBuilder::BuildFilledNewArray(uint32_t dex_pc, uint32_t* args, uint32_t register_index) { HInstruction* length = graph_->GetIntConstant(number_of_vreg_arguments, dex_pc); - QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index) + bool finalizable; + QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index, &finalizable) ? kQuickAllocArrayWithAccessCheck : kQuickAllocArray; HInstruction* object = new (arena_) HNewArray(length, @@ -1635,9 +1636,9 @@ void HGraphBuilder::BuildTypeCheck(const Instruction& instruction, } } -bool HGraphBuilder::NeedsAccessCheck(uint32_t type_index) const { +bool HGraphBuilder::NeedsAccessCheck(uint32_t type_index, bool* finalizable) const { return !compiler_driver_->CanAccessInstantiableTypeWithoutChecks( - dex_compilation_unit_->GetDexMethodIndex(), *dex_file_, type_index); + dex_compilation_unit_->GetDexMethodIndex(), *dex_file_, type_index, finalizable); } void HGraphBuilder::BuildSwitchJumpTable(const SwitchTable& table, @@ -2514,7 +2515,9 @@ bool HGraphBuilder::AnalyzeDexInstruction(const Instruction& instruction, uint32 current_block_->AddInstruction(fake_string); UpdateLocal(register_index, fake_string, dex_pc); } else { - QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index) + bool finalizable; + bool can_throw = NeedsAccessCheck(type_index, &finalizable); + QuickEntrypointEnum entrypoint = can_throw ? kQuickAllocObjectWithAccessCheck : kQuickAllocObject; @@ -2523,6 +2526,8 @@ bool HGraphBuilder::AnalyzeDexInstruction(const Instruction& instruction, uint32 dex_pc, type_index, *dex_compilation_unit_->GetDexFile(), + can_throw, + finalizable, entrypoint)); UpdateLocal(instruction.VRegA(), current_block_->GetLastInstruction(), dex_pc); } @@ -2532,7 +2537,8 @@ bool HGraphBuilder::AnalyzeDexInstruction(const Instruction& instruction, uint32 case Instruction::NEW_ARRAY: { uint16_t type_index = instruction.VRegC_22c(); HInstruction* length = LoadLocal(instruction.VRegB_22c(), Primitive::kPrimInt, dex_pc); - QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index) + bool finalizable; + QuickEntrypointEnum entrypoint = NeedsAccessCheck(type_index, &finalizable) ? kQuickAllocArrayWithAccessCheck : kQuickAllocArray; current_block_->AddInstruction(new (arena_) HNewArray(length, diff --git a/compiler/optimizing/builder.h b/compiler/optimizing/builder.h index 9eaa4b62c5..0f64489d98 100644 --- a/compiler/optimizing/builder.h +++ b/compiler/optimizing/builder.h @@ -138,7 +138,7 @@ class HGraphBuilder : public ValueObject { 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) const; + bool NeedsAccessCheck(uint32_t type_index, bool* finalizable) 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 90f28e511e..aa9c315ddd 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -695,8 +695,12 @@ class LSEVisitor : public HGraphVisitor { } else { redundant_store = true; } - // TODO: eliminate the store if the singleton object is not finalizable. - redundant_store = false; + 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; + } } if (redundant_store) { removed_instructions_.push_back(instruction); @@ -834,7 +838,9 @@ class LSEVisitor : public HGraphVisitor { return; } if (!heap_location_collector_.MayDeoptimize() && - ref_info->IsSingletonAndNotReturned()) { + ref_info->IsSingletonAndNotReturned() && + !new_instance->IsFinalizable() && + !new_instance->CanThrow()) { // 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 6028d4b6fa..7ac39d1a8e 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -3595,10 +3595,14 @@ class HNewInstance : public HExpression<1> { 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); } @@ -3608,11 +3612,11 @@ class HNewInstance : public HExpression<1> { // Calls runtime so needs an environment. bool NeedsEnvironment() const OVERRIDE { return true; } - // It may throw when called on: - // - interfaces - // - abstract/innaccessible/unknown classes - // TODO: optimize when possible. - bool CanThrow() 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_; } bool CanBeNull() const OVERRIDE { return false; } @@ -3623,6 +3627,8 @@ class HNewInstance : public HExpression<1> { 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 c766aaa6c9..924ff6773f 100644 --- a/test/530-checker-lse/src/Main.java +++ b/test/530-checker-lse/src/Main.java @@ -22,7 +22,7 @@ class Circle { return radius * radius * Math.PI; } private double radius; -}; +} class TestClass { TestClass() { @@ -36,16 +36,29 @@ class TestClass { 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 { @@ -56,7 +69,7 @@ public class Main { /// CHECK-START: double Main.calcCircleArea(double) load_store_elimination (after) /// CHECK: NewInstance - /// CHECK: InstanceFieldSet + /// CHECK-NOT: InstanceFieldSet /// CHECK-NOT: InstanceFieldGet static double calcCircleArea(double radius) { @@ -117,7 +130,7 @@ public class Main { /// CHECK: InstanceFieldGet /// CHECK: InstanceFieldSet /// CHECK: NewInstance - /// CHECK: InstanceFieldSet + /// CHECK-NOT: InstanceFieldSet /// CHECK-NOT: InstanceFieldGet // A new allocation shouldn't alias with pre-existing values. @@ -223,7 +236,7 @@ public class Main { /// CHECK-START: int Main.test8() load_store_elimination (after) /// CHECK: NewInstance - /// CHECK: InstanceFieldSet + /// CHECK-NOT: InstanceFieldSet /// CHECK: InvokeVirtual /// CHECK-NOT: NullCheck /// CHECK-NOT: InstanceFieldGet @@ -381,8 +394,8 @@ public class Main { /// CHECK-START: int Main.test16() load_store_elimination (after) /// CHECK: NewInstance - /// CHECK-NOT: StaticFieldSet - /// CHECK-NOT: StaticFieldGet + /// CHECK-NOT: InstanceFieldSet + /// CHECK-NOT: InstanceFieldGet // Test inlined constructor. static int test16() { @@ -398,8 +411,8 @@ public class Main { /// CHECK-START: int Main.test17() load_store_elimination (after) /// CHECK: <<Const0:i\d+>> IntConstant 0 /// CHECK: NewInstance - /// CHECK-NOT: StaticFieldSet - /// CHECK-NOT: StaticFieldGet + /// CHECK-NOT: InstanceFieldSet + /// CHECK-NOT: InstanceFieldGet /// CHECK: Return [<<Const0>>] // Test getting default value. @@ -455,6 +468,55 @@ public class Main { 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); @@ -508,5 +570,6 @@ public class Main { float[] fa2 = { 1.8f }; assertFloatEquals(test19(fa1, fa2), 1.8f); assertFloatEquals(test20().i, 0); + testFinalizableByForcingGc(); } } |