diff options
-rw-r--r-- | compiler/optimizing/inliner.cc | 20 | ||||
-rw-r--r-- | compiler/optimizing/instruction_builder.cc | 14 | ||||
-rw-r--r-- | compiler/optimizing/nodes.h | 10 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_compiler.cc | 6 | ||||
-rw-r--r-- | compiler/optimizing/optimizing_unit_test.h | 4 | ||||
-rw-r--r-- | test/476-checker-ctor-memory-barrier/src/Main.java | 20 |
6 files changed, 27 insertions, 47 deletions
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 298ae5c847..66948ebf8c 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -1565,25 +1565,6 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction, /* verified_method */ nullptr, dex_cache); - 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()); - } - } - InvokeType invoke_type = invoke_instruction->GetInvokeType(); if (invoke_type == kInterface) { // We have statically resolved the dispatch. To please the class linker @@ -1596,7 +1577,6 @@ bool HInliner::TryBuildAndInlineHelper(HInvoke* invoke_instruction, graph_->GetArena(), callee_dex_file, method_index, - requires_ctor_barrier, compiler_driver_->GetInstructionSet(), invoke_type, graph_->IsDebuggable(), diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc index 88f67fae04..978c6a2d71 100644 --- a/compiler/optimizing/instruction_builder.cc +++ b/compiler/optimizing/instruction_builder.cc @@ -589,6 +589,11 @@ void HInstructionBuilder::Binop_22b(const Instruction& instruction, bool reverse } static bool RequiresConstructorBarrier(const DexCompilationUnit* cu, CompilerDriver* driver) { + // Can be null in unit tests only. + if (UNLIKELY(cu == nullptr)) { + return false; + } + Thread* self = Thread::Current(); return cu->IsConstructor() && driver->RequiresConstructorBarrier(self, cu->GetDexFile(), cu->GetClassDefIndex()); @@ -634,12 +639,9 @@ void HInstructionBuilder::BuildReturn(const Instruction& instruction, Primitive::Type type, uint32_t dex_pc) { if (type == Primitive::kPrimVoid) { - 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."; - } + // This may insert additional redundant constructor fences from the super constructors. + // TODO: remove redundant constructor fences (b/36656456). + if (RequiresConstructorBarrier(dex_compilation_unit_, compiler_driver_)) { AppendInstruction(new (arena_) HMemoryBarrier(kStoreStore, dex_pc)); } AppendInstruction(new (arena_) HReturnVoid(dex_pc)); diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 671f950aa6..c109369106 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -305,7 +305,6 @@ class HGraph : public ArenaObject<kArenaAllocGraph> { HGraph(ArenaAllocator* arena, const DexFile& dex_file, uint32_t method_idx, - bool should_generate_constructor_barrier, InstructionSet instruction_set, InvokeType invoke_type = kInvalidInvokeType, bool debuggable = false, @@ -332,7 +331,6 @@ class HGraph : public ArenaObject<kArenaAllocGraph> { method_idx_(method_idx), invoke_type_(invoke_type), in_ssa_form_(false), - should_generate_constructor_barrier_(should_generate_constructor_barrier), number_of_cha_guards_(0), instruction_set_(instruction_set), cached_null_constant_(nullptr), @@ -504,10 +502,6 @@ class HGraph : public ArenaObject<kArenaAllocGraph> { 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 @@ -701,8 +695,6 @@ class HGraph : public ArenaObject<kArenaAllocGraph> { // for non-SSA form (like the number of temporaries). bool in_ssa_form_; - const bool should_generate_constructor_barrier_; - // Number of CHA guards in the graph. Used to short-circuit the // CHA guard optimization pass when there is no CHA guard left. uint32_t number_of_cha_guards_; @@ -5073,7 +5065,7 @@ class HParameterValue FINAL : public HExpression<0> { const DexFile& GetDexFile() const { return dex_file_; } dex::TypeIndex GetTypeIndex() const { return type_index_; } uint8_t GetIndex() const { return index_; } - bool IsThis() const { return GetPackedFlag<kFlagIsThis>(); } + bool IsThis() const ATTRIBUTE_UNUSED { return GetPackedFlag<kFlagIsThis>(); } bool CanBeNull() const OVERRIDE { return GetPackedFlag<kFlagCanBeNull>(); } void SetCanBeNull(bool can_be_null) { SetPackedFlag<kFlagCanBeNull>(can_be_null); } diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index e542cbbe37..8aad539851 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -930,16 +930,10 @@ CodeGenerator* OptimizingCompiler::TryCompile(ArenaAllocator* arena, /* verified_method */ nullptr, dex_cache); - bool requires_barrier = dex_compilation_unit.IsConstructor() - && compiler_driver->RequiresConstructorBarrier(Thread::Current(), - dex_compilation_unit.GetDexFile(), - dex_compilation_unit.GetClassDefIndex()); - HGraph* graph = new (arena) HGraph( arena, dex_file, method_idx, - requires_barrier, compiler_driver->GetInstructionSet(), kInvalidInvokeType, compiler_driver->GetCompilerOptions().GetDebuggable(), diff --git a/compiler/optimizing/optimizing_unit_test.h b/compiler/optimizing/optimizing_unit_test.h index bf963b8996..1cdcbd2e9b 100644 --- a/compiler/optimizing/optimizing_unit_test.h +++ b/compiler/optimizing/optimizing_unit_test.h @@ -79,7 +79,9 @@ void RemoveSuspendChecks(HGraph* graph) { inline HGraph* CreateGraph(ArenaAllocator* allocator) { return new (allocator) HGraph( - allocator, *reinterpret_cast<DexFile*>(allocator->Alloc(sizeof(DexFile))), -1, false, + allocator, + *reinterpret_cast<DexFile*>(allocator->Alloc(sizeof(DexFile))), + /*method_idx*/-1, kRuntimeISA); } diff --git a/test/476-checker-ctor-memory-barrier/src/Main.java b/test/476-checker-ctor-memory-barrier/src/Main.java index 65486e9e41..330aa7416e 100644 --- a/test/476-checker-ctor-memory-barrier/src/Main.java +++ b/test/476-checker-ctor-memory-barrier/src/Main.java @@ -37,6 +37,7 @@ class ClassWithFinals { /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: ReturnVoid public ClassWithFinals() { + // Exactly one constructor barrier. x = 0; } @@ -45,7 +46,7 @@ class ClassWithFinals { /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: ReturnVoid public ClassWithFinals(int x) { - // This should have two barriers: + // This should have exactly two barriers: // - one for the constructor // - one for the `new` which should be inlined. obj = new ClassWithFinals(); @@ -62,6 +63,8 @@ class InheritFromClassWithFinals extends ClassWithFinals { /// CHECK-NOT: InvokeStaticOrDirect public InheritFromClassWithFinals() { // Should inline the super constructor. + // + // Exactly one constructor barrier here. } /// CHECK-START: void InheritFromClassWithFinals.<init>(boolean) register (after) @@ -77,6 +80,7 @@ class InheritFromClassWithFinals extends ClassWithFinals { /// CHECK-START: void InheritFromClassWithFinals.<init>(int) register (after) /// CHECK: MemoryBarrier kind:StoreStore /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK-NOT: MemoryBarrier kind:StoreStore /// CHECK: ReturnVoid /// CHECK-START: void InheritFromClassWithFinals.<init>(int) register (after) @@ -94,12 +98,13 @@ class HaveFinalsAndInheritFromClassWithFinals extends ClassWithFinals { /// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() register (after) /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: ReturnVoid /// CHECK-START: void HaveFinalsAndInheritFromClassWithFinals.<init>() register (after) /// CHECK-NOT: InvokeStaticOrDirect public HaveFinalsAndInheritFromClassWithFinals() { - // Should inline the super constructor and remove the memory barrier. + // Should inline the super constructor and keep the memory barrier. y = 0; } @@ -117,17 +122,19 @@ class HaveFinalsAndInheritFromClassWithFinals extends ClassWithFinals { /// CHECK: MemoryBarrier kind:StoreStore /// CHECK: MemoryBarrier kind:StoreStore /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: 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. + // Should inline the super constructor and keep keep both memory barriers. y = 0; - // Should inline new instance and keep one barrier. + // Should inline new instance and keep both memory barriers. new HaveFinalsAndInheritFromClassWithFinals(); - // Should inline new instance and keep one barrier. + // Should inline new instance and have exactly one barrier. new InheritFromClassWithFinals(); } } @@ -166,6 +173,7 @@ public class Main { /// CHECK-START: void Main.inlineNew2() register (after) /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: ReturnVoid /// CHECK-START: void Main.inlineNew2() register (after) @@ -177,6 +185,8 @@ public class Main { /// CHECK-START: void Main.inlineNew3() register (after) /// CHECK: MemoryBarrier kind:StoreStore /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore + /// CHECK: MemoryBarrier kind:StoreStore /// CHECK-NEXT: ReturnVoid /// CHECK-START: void Main.inlineNew3() register (after) |