Eliminate redundant constructor barriers when inlining.
Bug: 20410297
Change-Id: I2097743d00eb795d050d390b1918e38c7f41d506
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index a5c6f23..c4eaabf 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -539,11 +539,6 @@
}
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 @@
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 afffc7a..56d868f 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -169,10 +169,30 @@
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 cb2e5cc..0291992 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -120,6 +120,7 @@
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 @@
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 @@
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 @@
// 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 @@
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 8bb5d8e..be9a424 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -512,9 +512,14 @@
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 4f8ec65..1fe9346 100644
--- a/compiler/optimizing/optimizing_unit_test.h
+++ b/compiler/optimizing/optimizing_unit_test.h
@@ -74,7 +74,7 @@
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 769ae20..9badc0f 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 @@
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 @@
// 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 @@
// 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 @@
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: ClassWithFinals Main.inlineConstructorBarrier() register (after)
+ // CHECK-START: void Main.inlineNew() register (after)
// CHECK-NOT: InvokeStaticOrDirect
- public static ClassWithFinals inlineConstructorBarrier() {
- return new ClassWithFinals();
+ public static void inlineNew() {
+ new ClassWithFinals();
}
- // CHECK-START: InheritFromClassWithFinals Main.doubleInlineConstructorBarrier() register (after)
+ // CHECK-START: void Main.inlineNew1() register (after)
// CHECK: MemoryBarrier kind:StoreStore
// CHECK-NOT: {{.*}}
// CHECK: Return
- // CHECK-START: InheritFromClassWithFinals Main.doubleInlineConstructorBarrier() register (after)
+ // CHECK-START: void Main.inlineNew1() register (after)
// CHECK-NOT: InvokeStaticOrDirect
- public static InheritFromClassWithFinals doubleInlineConstructorBarrier() {
- return new InheritFromClassWithFinals();
+ public static void inlineNew1() {
+ new InheritFromClassWithFinals();
}
- public static void main(String[] args) { }
+ // 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: void Main.inlineNew3() register (after)
+ // CHECK-NOT: InvokeStaticOrDirect
+ public static void inlineNew3() {
+ new HaveFinalsAndInheritFromClassWithFinals();
+ new HaveFinalsAndInheritFromClassWithFinals();
+ }
+
+ public static void main(String[] args) {}
}
