diff options
-rw-r--r-- | compiler/optimizing/builder.cc | 14 | ||||
-rw-r--r-- | compiler/optimizing/inliner.cc | 20 | ||||
-rw-r--r-- | compiler/optimizing/nodes.h | 10 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_compiler.cc | 7 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_unit_test.h | 2 | ||||
-rw-r--r-- | test/476-checker-ctor-memory-barrier/src/Main.java | 79 |
6 files changed, 111 insertions, 21 deletions
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc index a5c6f23343..c4eaabf899 100644 --- a/compiler/optimizing/builder.cc +++ b/compiler/optimizing/builder.cc @@ -539,11 +539,6 @@ void HGraphBuilder::Binop_22b(const Instruction& instruction, bool reverse) { } static bool RequiresConstructorBarrier(const DexCompilationUnit* cu, const CompilerDriver& driver) { - // dex compilation unit is null only when unit testing. - if (cu == nullptr) { - return false; - } - Thread* self = Thread::Current(); return cu->IsConstructor() && driver.RequiresConstructorBarrier(self, cu->GetDexFile(), cu->GetClassDefIndex()); @@ -551,9 +546,12 @@ static bool RequiresConstructorBarrier(const DexCompilationUnit* cu, const Compi void HGraphBuilder::BuildReturn(const Instruction& instruction, Primitive::Type type) { if (type == Primitive::kPrimVoid) { - // Note that we might insert redundant barriers when inlining `super` calls. - // TODO: add a data flow analysis to get rid of duplicate barriers. - if (RequiresConstructorBarrier(dex_compilation_unit_, *compiler_driver_)) { + if (graph_->ShouldGenerateConstructorBarrier()) { + // The compilation unit is null during testing. + if (dex_compilation_unit_ != nullptr) { + DCHECK(RequiresConstructorBarrier(dex_compilation_unit_, *compiler_driver_)) + << "Inconsistent use of ShouldGenerateConstructorBarrier. Should not generate a barrier."; + } current_block_->AddInstruction(new (arena_) HMemoryBarrier(kStoreStore)); } current_block_->AddInstruction(new (arena_) HReturnVoid()); diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index afffc7ab4f..56d868f116 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -169,10 +169,30 @@ bool HInliner::TryBuildAndInline(Handle<mirror::ArtMethod> resolved_method, resolved_method->GetAccessFlags(), nullptr); + bool requires_ctor_barrier = false; + + if (dex_compilation_unit.IsConstructor()) { + // If it's a super invocation and we already generate a barrier there's no need + // to generate another one. + // We identify super calls by looking at the "this" pointer. If its value is the + // same as the local "this" pointer then we must have a super invocation. + bool is_super_invocation = invoke_instruction->InputAt(0)->IsParameterValue() + && invoke_instruction->InputAt(0)->AsParameterValue()->IsThis(); + if (is_super_invocation && graph_->ShouldGenerateConstructorBarrier()) { + requires_ctor_barrier = false; + } else { + Thread* self = Thread::Current(); + requires_ctor_barrier = compiler_driver_->RequiresConstructorBarrier(self, + dex_compilation_unit.GetDexFile(), + dex_compilation_unit.GetClassDefIndex()); + } + } + HGraph* callee_graph = new (graph_->GetArena()) HGraph( graph_->GetArena(), caller_dex_file, method_index, + requires_ctor_barrier, graph_->IsDebuggable(), graph_->GetCurrentInstructionId()); diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index cb2e5ccab4..0291992cab 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -120,6 +120,7 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { HGraph(ArenaAllocator* arena, const DexFile& dex_file, uint32_t method_idx, + bool should_generate_constructor_barrier, bool debuggable = false, int start_instruction_id = 0) : arena_(arena), @@ -137,6 +138,7 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { current_instruction_id_(start_instruction_id), dex_file_(dex_file), method_idx_(method_idx), + should_generate_constructor_barrier_(should_generate_constructor_barrier), cached_null_constant_(nullptr), cached_int_constants_(std::less<int32_t>(), arena->Adapter()), cached_float_constants_(std::less<int32_t>(), arena->Adapter()), @@ -247,6 +249,10 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { has_bounds_checks_ = value; } + bool ShouldGenerateConstructorBarrier() const { + return should_generate_constructor_barrier_; + } + bool IsDebuggable() const { return debuggable_; } // Returns a constant of the given type and value. If it does not exist @@ -359,6 +365,8 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { // The method index in the dex file. const uint32_t method_idx_; + const bool should_generate_constructor_barrier_; + // Cached constants. HNullConstant* cached_null_constant_; ArenaSafeMap<int32_t, HIntConstant*> cached_int_constants_; @@ -2902,6 +2910,8 @@ class HParameterValue : public HExpression<0> { bool CanBeNull() const OVERRIDE { return !is_this_; } + bool IsThis() const { return is_this_; } + DECLARE_INSTRUCTION(ParameterValue); private: diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index 8bb5d8ebae..be9a42425b 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -512,9 +512,14 @@ CompiledMethod* OptimizingCompiler::TryCompile(const DexFile::CodeItem* code_ite class_def_idx, method_idx, access_flags, compiler_driver->GetVerifiedMethod(&dex_file, method_idx)); + bool requires_barrier = dex_compilation_unit.IsConstructor() + && compiler_driver->RequiresConstructorBarrier(Thread::Current(), + dex_compilation_unit.GetDexFile(), + dex_compilation_unit.GetClassDefIndex()); ArenaAllocator arena(Runtime::Current()->GetArenaPool()); HGraph* graph = new (&arena) HGraph( - &arena, dex_file, method_idx, compiler_driver->GetCompilerOptions().GetDebuggable()); + &arena, dex_file, method_idx, requires_barrier, + compiler_driver->GetCompilerOptions().GetDebuggable()); // For testing purposes, we put a special marker on method names that should be compiled // with this compiler. This makes sure we're not regressing. diff --git a/compiler/optimizing/optimizing_unit_test.h b/compiler/optimizing/optimizing_unit_test.h index 4f8ec65e43..1fe9346b0f 100644 --- a/compiler/optimizing/optimizing_unit_test.h +++ b/compiler/optimizing/optimizing_unit_test.h @@ -74,7 +74,7 @@ void RemoveSuspendChecks(HGraph* graph) { inline HGraph* CreateGraph(ArenaAllocator* allocator) { return new (allocator) HGraph( - allocator, *reinterpret_cast<DexFile*>(allocator->Alloc(sizeof(DexFile))), -1); + allocator, *reinterpret_cast<DexFile*>(allocator->Alloc(sizeof(DexFile))), -1, false); } // Create a control-flow graph from Dex instructions. diff --git a/test/476-checker-ctor-memory-barrier/src/Main.java b/test/476-checker-ctor-memory-barrier/src/Main.java index 769ae208d9..9badc0f5e0 100644 --- a/test/476-checker-ctor-memory-barrier/src/Main.java +++ b/test/476-checker-ctor-memory-barrier/src/Main.java @@ -14,6 +14,7 @@ * limitations under the License. */ +// TODO: Add more tests after we can inline functions with calls. class ClassWithoutFinals { // CHECK-START: void ClassWithoutFinals.<init>() register (after) @@ -80,6 +81,20 @@ class InheritFromClassWithFinals extends ClassWithFinals { super(cond); // should not inline the super constructor } + + // CHECK-START: void InheritFromClassWithFinals.<init>(int) register (after) + // CHECK: MemoryBarrier kind:StoreStore + // CHECK: MemoryBarrier kind:StoreStore + // CHECK: ReturnVoid + + // CHECK-START: void InheritFromClassWithFinals.<init>(int) register (after) + // CHECK-NOT: InvokeStaticOrDirect + public InheritFromClassWithFinals(int unused) { + // Should inline the super constructor and insert a memory barrier. + + // Should inline the new instance call and insert another memory barrier. + new InheritFromClassWithFinals(); + } } class HaveFinalsAndInheritFromClassWithFinals extends ClassWithFinals { @@ -87,14 +102,13 @@ class HaveFinalsAndInheritFromClassWithFinals extends ClassWithFinals { // CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() register (after) // CHECK: MemoryBarrier kind:StoreStore - // CHECK: MemoryBarrier kind:StoreStore // CHECK-NOT: {{.*}} // CHECK: ReturnVoid // CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() register (after) // CHECK-NOT: InvokeStaticOrDirect public HaveFinalsAndInheritFromClassWithFinals() { - // Should inline the super constructor. + // Should inline the super constructor and remove the memory barrier. y = 0; } @@ -108,6 +122,25 @@ class HaveFinalsAndInheritFromClassWithFinals extends ClassWithFinals { // should not inline the super constructor y = 0; } + + // CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>(int) register (after) + // CHECK: MemoryBarrier kind:StoreStore + // CHECK: MemoryBarrier kind:StoreStore + // CHECK: MemoryBarrier kind:StoreStore + // CHECK-NOT: {{.*}} + // CHECK: ReturnVoid + + // CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>(int) register (after) + // CHECK-NOT: InvokeStaticOrDirect + public HaveFinalsAndInheritFromClassWithFinals(int unused) { + // Should inline the super constructor and keep just one memory barrier. + y = 0; + + // Should inline new instance and keep one barrier. + new HaveFinalsAndInheritFromClassWithFinals(); + // Should inline new instance and keep one barrier. + new InheritFromClassWithFinals(); + } } public class Main { @@ -121,27 +154,51 @@ public class Main { return new ClassWithFinals(false); } - // CHECK-START: ClassWithFinals Main.inlineConstructorBarrier() register (after) + // CHECK-START: void Main.inlineNew() register (after) + // CHECK: MemoryBarrier kind:StoreStore + // CHECK-NOT: {{.*}} + // CHECK: Return + + // CHECK-START: void Main.inlineNew() register (after) + // CHECK-NOT: InvokeStaticOrDirect + public static void inlineNew() { + new ClassWithFinals(); + } + + // CHECK-START: void Main.inlineNew1() register (after) // CHECK: MemoryBarrier kind:StoreStore // CHECK-NOT: {{.*}} // CHECK: Return - // CHECK-START: ClassWithFinals Main.inlineConstructorBarrier() register (after) + // CHECK-START: void Main.inlineNew1() register (after) // CHECK-NOT: InvokeStaticOrDirect - public static ClassWithFinals inlineConstructorBarrier() { - return new ClassWithFinals(); + public static void inlineNew1() { + new InheritFromClassWithFinals(); } - // CHECK-START: InheritFromClassWithFinals Main.doubleInlineConstructorBarrier() register (after) + // CHECK-START: void Main.inlineNew2() register (after) + // CHECK: MemoryBarrier kind:StoreStore + // CHECK-NOT: {{.*}} + // CHECK: Return + + // CHECK-START: void Main.inlineNew2() register (after) + // CHECK-NOT: InvokeStaticOrDirect + public static void inlineNew2() { + new HaveFinalsAndInheritFromClassWithFinals(); + } + + // CHECK-START: void Main.inlineNew3() register (after) + // CHECK: MemoryBarrier kind:StoreStore // CHECK: MemoryBarrier kind:StoreStore // CHECK-NOT: {{.*}} // CHECK: Return - // CHECK-START: InheritFromClassWithFinals Main.doubleInlineConstructorBarrier() register (after) + // CHECK-START: void Main.inlineNew3() register (after) // CHECK-NOT: InvokeStaticOrDirect - public static InheritFromClassWithFinals doubleInlineConstructorBarrier() { - return new InheritFromClassWithFinals(); + public static void inlineNew3() { + new HaveFinalsAndInheritFromClassWithFinals(); + new HaveFinalsAndInheritFromClassWithFinals(); } - public static void main(String[] args) { } + public static void main(String[] args) {} } |