Support for inlining methods that call/throw.
Mostly fixes here and there to make it working.
Change-Id: I1b535e895105d78b65634636d675b818551f783e
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index f858f82..365599f 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -652,8 +652,8 @@
DCHECK((optimized_invoke_type == invoke_type) || (optimized_invoke_type != kDirect)
|| compiler_driver_->GetCompilerOptions().GetCompilePic());
bool is_recursive =
- (target_method.dex_method_index == dex_compilation_unit_->GetDexMethodIndex());
- DCHECK(!is_recursive || (target_method.dex_file == dex_compilation_unit_->GetDexFile()));
+ (target_method.dex_method_index == outer_compilation_unit_->GetDexMethodIndex())
+ && (target_method.dex_file == outer_compilation_unit_->GetDexFile());
if (optimized_invoke_type == kStatic) {
ScopedObjectAccess soa(Thread::Current());
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 7da4f2d..fd2e4e8 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -280,6 +280,13 @@
<< instance_of->MustDoNullCheck() << std::noboolalpha;
}
+ void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE {
+ StartAttributeStream("dex_file_index") << invoke->GetDexMethodIndex();
+ StartAttributeStream("recursive") << std::boolalpha
+ << invoke->IsRecursive()
+ << std::noboolalpha;
+ }
+
bool IsPass(const char* name) {
return strcmp(pass_name_, name) == 0;
}
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 997f980..15f3deb 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -36,8 +36,8 @@
namespace art {
-static constexpr int kMaxInlineCodeUnits = 100;
-static constexpr int kDepthLimit = 5;
+static constexpr int kMaxInlineCodeUnits = 18;
+static constexpr int kDepthLimit = 3;
void HInliner::Run() {
if (graph_->IsDebuggable()) {
@@ -46,8 +46,15 @@
return;
}
const GrowableArray<HBasicBlock*>& blocks = graph_->GetReversePostOrder();
+ HBasicBlock* next_block = blocks.Get(0);
for (size_t i = 0; i < blocks.Size(); ++i) {
- HBasicBlock* block = blocks.Get(i);
+ // Because we are changing the graph when inlining, we need to remember the next block.
+ // This avoids doing the inlining work again on the inlined blocks.
+ if (blocks.Get(i) != next_block) {
+ continue;
+ }
+ HBasicBlock* block = next_block;
+ next_block = (i == blocks.Size() - 1) ? nullptr : blocks.Get(i + 1);
for (HInstruction* instruction = block->GetFirstInstruction(); instruction != nullptr;) {
HInstruction* next = instruction->GetNext();
HInvokeStaticOrDirect* call = instruction->AsInvokeStaticOrDirect();
@@ -90,10 +97,10 @@
return false;
}
- bool can_use_dex_cache = true;
+ bool same_dex_file = true;
const DexFile& outer_dex_file = *outer_compilation_unit_.GetDexFile();
if (resolved_method->GetDexFile()->GetLocation().compare(outer_dex_file.GetLocation()) != 0) {
- can_use_dex_cache = false;
+ same_dex_file = false;
}
const DexFile::CodeItem* code_item = resolved_method->GetCodeItem();
@@ -140,7 +147,7 @@
return false;
}
- if (!TryBuildAndInline(resolved_method, invoke_instruction, method_index, can_use_dex_cache)) {
+ if (!TryBuildAndInline(resolved_method, invoke_instruction, method_index, same_dex_file)) {
return false;
}
@@ -152,7 +159,7 @@
bool HInliner::TryBuildAndInline(Handle<mirror::ArtMethod> resolved_method,
HInvoke* invoke_instruction,
uint32_t method_index,
- bool can_use_dex_cache) const {
+ bool same_dex_file) const {
ScopedObjectAccess soa(Thread::Current());
const DexFile::CodeItem* code_item = resolved_method->GetCodeItem();
const DexFile& caller_dex_file = *caller_compilation_unit_.GetDexFile();
@@ -254,6 +261,31 @@
inliner.Run();
}
+ // TODO: We should abort only if all predecessors throw. However,
+ // HGraph::InlineInto currently does not handle an exit block with
+ // a throw predecessor.
+ HBasicBlock* exit_block = callee_graph->GetExitBlock();
+ if (exit_block == nullptr) {
+ VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
+ << " could not be inlined because it has an infinite loop";
+ resolved_method->SetShouldNotInline();
+ return false;
+ }
+
+ bool has_throw_predecessor = false;
+ for (size_t i = 0, e = exit_block->GetPredecessors().Size(); i < e; ++i) {
+ if (exit_block->GetPredecessors().Get(i)->GetLastInstruction()->IsThrow()) {
+ has_throw_predecessor = true;
+ break;
+ }
+ }
+ if (has_throw_predecessor) {
+ VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
+ << " could not be inlined because one branch always throws";
+ resolved_method->SetShouldNotInline();
+ return false;
+ }
+
HReversePostOrderIterator it(*callee_graph);
it.Advance(); // Past the entry block, it does not contain instructions that prevent inlining.
for (; !it.Done(); it.Advance()) {
@@ -269,27 +301,24 @@
!instr_it.Done();
instr_it.Advance()) {
HInstruction* current = instr_it.Current();
- if (current->IsSuspendCheck()) {
- continue;
- }
- if (current->CanThrow()) {
+ if (current->IsInvokeInterface()) {
+ // Disable inlining of interface calls. The cost in case of entering the
+ // resolution conflict is currently too high.
VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
- << " could not be inlined because " << current->DebugName()
- << " can throw";
+ << " could not be inlined because it has an interface call.";
resolved_method->SetShouldNotInline();
return false;
}
- if (current->NeedsEnvironment()) {
+ if (!same_dex_file && current->NeedsEnvironment()) {
VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
<< " could not be inlined because " << current->DebugName()
- << " needs an environment";
- resolved_method->SetShouldNotInline();
+ << " needs an environment and is in a different dex file";
return false;
}
- if (!can_use_dex_cache && current->NeedsDexCache()) {
+ if (!same_dex_file && current->NeedsDexCache()) {
VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file)
<< " could not be inlined because " << current->DebugName()
<< " it is in a different dex file and requires access to the dex cache";
@@ -302,10 +331,6 @@
callee_graph->InlineInto(graph_, invoke_instruction);
- if (callee_graph->HasBoundsChecks()) {
- graph_->SetHasBoundsChecks(true);
- }
-
return true;
}
diff --git a/compiler/optimizing/inliner.h b/compiler/optimizing/inliner.h
index 1dbc7d3..09a36c6 100644
--- a/compiler/optimizing/inliner.h
+++ b/compiler/optimizing/inliner.h
@@ -51,7 +51,7 @@
bool TryBuildAndInline(Handle<mirror::ArtMethod> resolved_method,
HInvoke* invoke_instruction,
uint32_t method_index,
- bool can_use_dex_cache) const;
+ bool same_dex_file) const;
const DexCompilationUnit& outer_compilation_unit_;
const DexCompilationUnit& caller_compilation_unit_;
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 80d4b4a..06f6a7f 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1314,6 +1314,29 @@
void HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) {
DCHECK(HasExitBlock()) << "Unimplemented scenario";
+ // Update the environments in this graph to have the invoke's environment
+ // as parent.
+ {
+ HReversePostOrderIterator it(*this);
+ it.Advance(); // Skip the entry block, we do not need to update the entry's suspend check.
+ for (; !it.Done(); it.Advance()) {
+ HBasicBlock* block = it.Current();
+ for (HInstructionIterator instr_it(block->GetInstructions());
+ !instr_it.Done();
+ instr_it.Advance()) {
+ HInstruction* current = instr_it.Current();
+ if (current->NeedsEnvironment()) {
+ current->GetEnvironment()->SetAndCopyParentChain(
+ outer_graph->GetArena(), invoke->GetEnvironment());
+ }
+ }
+ }
+ }
+ outer_graph->UpdateMaximumNumberOfOutVRegs(GetMaximumNumberOfOutVRegs());
+ if (HasBoundsChecks()) {
+ outer_graph->SetHasBoundsChecks(true);
+ }
+
if (GetBlocks().Size() == 3) {
// Simple case of an entry block, a body block, and an exit block.
// Put the body block's instruction into `invoke`'s block.
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 3144c5c..005d50e 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -143,6 +143,7 @@
dex_file_(dex_file),
method_idx_(method_idx),
invoke_type_(invoke_type),
+ in_ssa_form_(false),
should_generate_constructor_barrier_(should_generate_constructor_barrier),
cached_null_constant_(nullptr),
cached_int_constants_(std::less<int32_t>(), arena->Adapter()),
@@ -174,6 +175,7 @@
// users remaining when being visited.
if (!AnalyzeNaturalLoops()) return false;
TransformToSsa();
+ in_ssa_form_ = true;
return true;
}
@@ -216,11 +218,16 @@
maximum_number_of_out_vregs_ = new_value;
}
+ void UpdateMaximumNumberOfOutVRegs(uint16_t other_value) {
+ maximum_number_of_out_vregs_ = std::max(maximum_number_of_out_vregs_, other_value);
+ }
+
void UpdateTemporariesVRegSlots(size_t slots) {
temporaries_vreg_slots_ = std::max(slots, temporaries_vreg_slots_);
}
size_t GetTemporariesVRegSlots() const {
+ DCHECK(!in_ssa_form_);
return temporaries_vreg_slots_;
}
@@ -229,6 +236,7 @@
}
uint16_t GetNumberOfVRegs() const {
+ DCHECK(!in_ssa_form_);
return number_of_vregs_;
}
@@ -237,6 +245,7 @@
}
uint16_t GetNumberOfLocalVRegs() const {
+ DCHECK(!in_ssa_form_);
return number_of_vregs_ - number_of_in_vregs_;
}
@@ -381,6 +390,11 @@
// If inlined, this encodes how the callee is being invoked.
const InvokeType invoke_type_;
+ // Whether the graph has been transformed to SSA form. Only used
+ // in debug mode to ensure we are not using properties only valid
+ // for non-SSA form (like the number of temporaries).
+ bool in_ssa_form_;
+
const bool should_generate_constructor_barrier_;
// Cached constants.
@@ -1121,14 +1135,16 @@
const DexFile& dex_file,
uint32_t method_idx,
uint32_t dex_pc,
- InvokeType invoke_type)
+ InvokeType invoke_type,
+ HInstruction* holder)
: vregs_(arena, number_of_vregs),
locations_(arena, number_of_vregs),
parent_(nullptr),
dex_file_(dex_file),
method_idx_(method_idx),
dex_pc_(dex_pc),
- invoke_type_(invoke_type) {
+ invoke_type_(invoke_type),
+ holder_(holder) {
vregs_.SetSize(number_of_vregs);
for (size_t i = 0; i < number_of_vregs; i++) {
vregs_.Put(i, HUserRecord<HEnvironment*>());
@@ -1140,19 +1156,24 @@
}
}
- HEnvironment(ArenaAllocator* arena, const HEnvironment& to_copy)
+ HEnvironment(ArenaAllocator* arena, const HEnvironment& to_copy, HInstruction* holder)
: HEnvironment(arena,
to_copy.Size(),
to_copy.GetDexFile(),
to_copy.GetMethodIdx(),
to_copy.GetDexPc(),
- to_copy.GetInvokeType()) {}
+ to_copy.GetInvokeType(),
+ holder) {}
void SetAndCopyParentChain(ArenaAllocator* allocator, HEnvironment* parent) {
- parent_ = new (allocator) HEnvironment(allocator, *parent);
- parent_->CopyFrom(parent);
- if (parent->GetParent() != nullptr) {
- parent_->SetAndCopyParentChain(allocator, parent->GetParent());
+ if (parent_ != nullptr) {
+ parent_->SetAndCopyParentChain(allocator, parent);
+ } else {
+ parent_ = new (allocator) HEnvironment(allocator, *parent, holder_);
+ parent_->CopyFrom(parent);
+ if (parent->GetParent() != nullptr) {
+ parent_->SetAndCopyParentChain(allocator, parent->GetParent());
+ }
}
}
@@ -1202,6 +1223,10 @@
return dex_file_;
}
+ HInstruction* GetHolder() const {
+ return holder_;
+ }
+
private:
// Record instructions' use entries of this environment for constant-time removal.
// It should only be called by HInstruction when a new environment use is added.
@@ -1219,6 +1244,10 @@
const uint32_t dex_pc_;
const InvokeType invoke_type_;
+ // The instruction that holds this environment. Only used in debug mode
+ // to ensure the graph is consistent.
+ HInstruction* const holder_;
+
friend class HInstruction;
DISALLOW_COPY_AND_ASSIGN(HEnvironment);
@@ -1425,13 +1454,18 @@
HEnvironment* GetEnvironment() const { return environment_; }
// Set the `environment_` field. Raw because this method does not
// update the uses lists.
- void SetRawEnvironment(HEnvironment* environment) { environment_ = environment; }
+ void SetRawEnvironment(HEnvironment* environment) {
+ DCHECK(environment_ == nullptr);
+ DCHECK_EQ(environment->GetHolder(), this);
+ environment_ = environment;
+ }
// Set the environment of this instruction, copying it from `environment`. While
// copying, the uses lists are being updated.
void CopyEnvironmentFrom(HEnvironment* environment) {
+ DCHECK(environment_ == nullptr);
ArenaAllocator* allocator = GetBlock()->GetGraph()->GetArena();
- environment_ = new (allocator) HEnvironment(allocator, *environment);
+ environment_ = new (allocator) HEnvironment(allocator, *environment, this);
environment_->CopyFrom(environment);
if (environment->GetParent() != nullptr) {
environment_->SetAndCopyParentChain(allocator, environment->GetParent());
@@ -1440,8 +1474,9 @@
void CopyEnvironmentFromWithLoopPhiAdjustment(HEnvironment* environment,
HBasicBlock* block) {
+ DCHECK(environment_ == nullptr);
ArenaAllocator* allocator = GetBlock()->GetGraph()->GetArena();
- environment_ = new (allocator) HEnvironment(allocator, *environment);
+ environment_ = new (allocator) HEnvironment(allocator, *environment, this);
environment_->CopyFromWithLoopPhiAdjustment(environment, block);
if (environment->GetParent() != nullptr) {
environment_->SetAndCopyParentChain(allocator, environment->GetParent());
@@ -2420,6 +2455,12 @@
intrinsic_ = intrinsic;
}
+ bool IsInlined() const {
+ return GetEnvironment()->GetParent() != nullptr;
+ }
+
+ bool CanThrow() const OVERRIDE { return true; }
+
DECLARE_INSTRUCTION(Invoke);
protected:
diff --git a/compiler/optimizing/nodes_test.cc b/compiler/optimizing/nodes_test.cc
index 782cde4..fef77aa 100644
--- a/compiler/optimizing/nodes_test.cc
+++ b/compiler/optimizing/nodes_test.cc
@@ -51,7 +51,7 @@
exit_block->AddInstruction(new (&allocator) HExit());
HEnvironment* environment = new (&allocator) HEnvironment(
- &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic);
+ &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic, null_check);
null_check->SetRawEnvironment(environment);
environment->SetRawEnvAt(0, parameter);
parameter->AddEnvUseAt(null_check->GetEnvironment(), 0);
@@ -132,7 +132,7 @@
ASSERT_TRUE(parameter1->GetUses().HasOnlyOneUse());
HEnvironment* environment = new (&allocator) HEnvironment(
- &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic);
+ &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic, with_environment);
GrowableArray<HInstruction*> array(&allocator, 1);
array.Add(parameter1);
@@ -143,13 +143,13 @@
ASSERT_TRUE(parameter1->GetEnvUses().HasOnlyOneUse());
HEnvironment* parent1 = new (&allocator) HEnvironment(
- &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic);
+ &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic, nullptr);
parent1->CopyFrom(array);
ASSERT_EQ(parameter1->GetEnvUses().SizeSlow(), 2u);
HEnvironment* parent2 = new (&allocator) HEnvironment(
- &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic);
+ &allocator, 1, graph->GetDexFile(), graph->GetMethodIdx(), 0, kStatic, nullptr);
parent2->CopyFrom(array);
parent1->SetAndCopyParentChain(&allocator, parent2);
diff --git a/compiler/optimizing/prepare_for_register_allocation.cc b/compiler/optimizing/prepare_for_register_allocation.cc
index 538736b..a249aa9 100644
--- a/compiler/optimizing/prepare_for_register_allocation.cc
+++ b/compiler/optimizing/prepare_for_register_allocation.cc
@@ -88,7 +88,11 @@
// The static call will initialize the class so there's no need for a clinit check if
// it's the first user.
- if (last_input == invoke->GetPrevious()) {
+ // There is one special case where we still need the clinit check, when inlining. Because
+ // currently the callee is responsible for reporting parameters to the GC, the code
+ // that walks the stack during `artQuickResolutionTrampoline` cannot be interrupted for GC.
+ // Therefore we cannot allocate any object in that code, including loading a new class.
+ if (last_input == invoke->GetPrevious() && !invoke->IsInlined()) {
last_input->SetMustGenerateClinitCheck(false);
}
@@ -102,7 +106,7 @@
// If the load class instruction is no longer used, remove it from
// the graph.
- if (!last_input->HasUses()) {
+ if (!last_input->HasUses() && !(last_input->MustGenerateClinitCheck() && invoke->IsInlined())) {
last_input->GetBlock()->RemoveInstruction(last_input);
}
}
diff --git a/compiler/optimizing/register_allocator.cc b/compiler/optimizing/register_allocator.cc
index d4ff4d8..9a859bf 100644
--- a/compiler/optimizing/register_allocator.cc
+++ b/compiler/optimizing/register_allocator.cc
@@ -1583,7 +1583,7 @@
while (env_use != nullptr && env_use->GetPosition() <= range->GetEnd()) {
DCHECK(current->CoversSlow(env_use->GetPosition())
|| (env_use->GetPosition() == range->GetEnd()));
- HEnvironment* environment = env_use->GetUser()->GetEnvironment();
+ HEnvironment* environment = env_use->GetEnvironment();
environment->SetLocationAt(env_use->GetInputIndex(), source);
env_use = env_use->GetNext();
}
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index c51d248..c4612af 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -548,7 +548,8 @@
GetGraph()->GetDexFile(),
GetGraph()->GetMethodIdx(),
instruction->GetDexPc(),
- GetGraph()->GetInvokeType());
+ GetGraph()->GetInvokeType(),
+ instruction);
environment->CopyFrom(*current_locals_);
instruction->SetRawEnvironment(environment);
}
diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h
index 4b19c5b..4cbe29a 100644
--- a/compiler/optimizing/ssa_liveness_analysis.h
+++ b/compiler/optimizing/ssa_liveness_analysis.h
@@ -117,6 +117,7 @@
|| user->IsPhi()
|| (GetPosition() == user->GetLifetimePosition() + 1)
|| (GetPosition() == user->GetLifetimePosition()));
+ DCHECK(environment == nullptr || user == nullptr);
DCHECK(next_ == nullptr || next->GetPosition() >= GetPosition());
}
@@ -128,6 +129,7 @@
void SetNext(UsePosition* next) { next_ = next; }
HInstruction* GetUser() const { return user_; }
+ HEnvironment* GetEnvironment() const { return environment_; }
bool GetIsEnvironment() const { return environment_ != nullptr; }
bool IsSynthesized() const { return user_ == nullptr; }
@@ -280,7 +282,7 @@
}
DCHECK(first_use_->GetPosition() + 1 == position);
UsePosition* new_use = new (allocator_) UsePosition(
- instruction, environment, input_index, position, cursor->GetNext());
+ instruction, nullptr /* environment */, input_index, position, cursor->GetNext());
cursor->SetNext(new_use);
if (first_range_->GetEnd() == first_use_->GetPosition()) {
first_range_->end_ = position;
@@ -290,10 +292,10 @@
if (is_environment) {
first_env_use_ = new (allocator_) UsePosition(
- instruction, environment, input_index, position, first_env_use_);
+ nullptr /* instruction */, environment, input_index, position, first_env_use_);
} else {
first_use_ = new (allocator_) UsePosition(
- instruction, environment, input_index, position, first_use_);
+ instruction, nullptr /* environment */, input_index, position, first_use_);
}
if (is_environment && !keep_alive) {