Inline across dex files for bootclaspath's methods
We can relax a bit the restriction for not inlining across dexfiles when
we are AoT compiling and we need an environment. There's an added new
restriction related to BSS entries. We could potentially inline across
dex files for those cases too but are left to be solved in follow-up
CLs.
Bug: 154012332
Test: ART tests
Change-Id: I5122b26c79b3e30d2643c0ccc05d595a0047953e
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 3abbbae..0e4f9ef 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1696,18 +1696,26 @@
return (object != hint.Get()) ? graph->GetHandleCache()->NewHandle(object) : hint;
}
-static bool CanEncodeInlinedMethodInStackMap(const DexFile& caller_dex_file, ArtMethod* callee)
+static bool CanEncodeInlinedMethodInStackMap(const DexFile& outer_dex_file,
+ ArtMethod* callee,
+ bool* out_needs_bss_check)
REQUIRES_SHARED(Locks::mutator_lock_) {
if (!Runtime::Current()->IsAotCompiler()) {
// JIT can always encode methods in stack maps.
return true;
}
- if (IsSameDexFile(caller_dex_file, *callee->GetDexFile())) {
+ if (IsSameDexFile(outer_dex_file, *callee->GetDexFile())) {
return true;
}
+
+ // Inline across dexfiles if the callee's DexFile is in the bootclasspath.
+ if (callee->GetDeclaringClass()->GetClassLoader() == nullptr) {
+ *out_needs_bss_check = true;
+ return true;
+ }
+
// TODO(ngeoffray): Support more AOT cases for inlining:
// - methods in multidex
- // - methods in boot image for on-device non-PIC compilation.
return false;
}
@@ -1817,6 +1825,11 @@
return false;
}
+ const bool too_many_registers =
+ total_number_of_dex_registers_ > kMaximumNumberOfCumulatedDexRegisters;
+ bool needs_bss_check = false;
+ const bool can_encode_in_stack_map = CanEncodeInlinedMethodInStackMap(
+ *outer_compilation_unit_.GetDexFile(), resolved_method, &needs_bss_check);
size_t number_of_instructions = 0;
// Skip the entry block, it does not contain instructions that prevent inlining.
for (HBasicBlock* block : callee_graph->GetReversePostOrderSkipEntryBlock()) {
@@ -1851,24 +1864,22 @@
return false;
}
HInstruction* current = instr_it.Current();
- if (current->NeedsEnvironment() &&
- (total_number_of_dex_registers_ > kMaximumNumberOfCumulatedDexRegisters)) {
- LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedEnvironmentBudget)
- << "Method " << resolved_method->PrettyMethod()
- << " is not inlined because its caller has reached"
- << " its environment budget limit.";
- return false;
- }
+ if (current->NeedsEnvironment()) {
+ if (too_many_registers) {
+ LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedEnvironmentBudget)
+ << "Method " << resolved_method->PrettyMethod()
+ << " is not inlined because its caller has reached"
+ << " its environment budget limit.";
+ return false;
+ }
- if (current->NeedsEnvironment() &&
- !CanEncodeInlinedMethodInStackMap(*caller_compilation_unit_.GetDexFile(),
- resolved_method)) {
- LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedStackMaps)
- << "Method " << resolved_method->PrettyMethod()
- << " could not be inlined because " << current->DebugName()
- << " needs an environment, is in a different dex file"
- << ", and cannot be encoded in the stack maps.";
- return false;
+ if (!can_encode_in_stack_map) {
+ LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedStackMaps)
+ << "Method " << resolved_method->PrettyMethod() << " could not be inlined because "
+ << current->DebugName() << " needs an environment, is in a different dex file"
+ << ", and cannot be encoded in the stack maps.";
+ return false;
+ }
}
if (current->IsUnresolvedStaticFieldGet() ||
@@ -1882,6 +1893,21 @@
<< " entrypoint";
return false;
}
+
+ // We currently don't have support for inlining across dex files if the inlined method needs a
+ // .bss entry. This only happens when we are:
+ // 1) In AoT,
+ // 2) cross-dex inlining, and
+ // 3) have an instruction that needs a bss entry, which will always be
+ // 3)b) an instruction that needs an environment.
+ // TODO(solanes, 154012332): Add this support.
+ if (needs_bss_check && current->NeedsBss()) {
+ DCHECK(current->NeedsEnvironment());
+ LOG_FAIL(stats_, MethodCompilationStat::kNotInlinedBss)
+ << "Method " << resolved_method->PrettyMethod()
+ << " could not be inlined because it needs a BSS check";
+ return false;
+ }
}
}
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 91b2e8b..3f1f3b0 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2645,12 +2645,15 @@
// Collect args and check for unexpected uses.
// We expect one call to a constructor with no arguments, one constructor fence (unless
// eliminated), some number of append calls and one call to StringBuilder.toString().
+ bool constructor_inlined = false;
bool seen_constructor = false;
bool seen_constructor_fence = false;
bool seen_to_string = false;
uint32_t format = 0u;
uint32_t num_args = 0u;
HInstruction* args[StringBuilderAppend::kMaxArgs]; // Added in reverse order.
+ // When inlining, `maybe_new_array` tracks an environment use that we want to allow.
+ HInstruction* maybe_new_array = nullptr;
for (HBackwardInstructionIterator iter(block->GetInstructions()); !iter.Done(); iter.Advance()) {
HInstruction* user = iter.Current();
// Instructions of interest apply to `sb`, skip those that do not involve `sb`.
@@ -2731,13 +2734,25 @@
format = (format << StringBuilderAppend::kBitsPerArg) | static_cast<uint32_t>(arg);
args[num_args] = as_invoke_virtual->InputAt(1u);
++num_args;
- } else if (user->IsInvokeStaticOrDirect() &&
- user->AsInvokeStaticOrDirect()->GetResolvedMethod() != nullptr &&
- user->AsInvokeStaticOrDirect()->GetResolvedMethod()->IsConstructor() &&
- user->AsInvokeStaticOrDirect()->GetNumberOfArguments() == 1u) {
- // After arguments, we should see the constructor.
- // We accept only the constructor with no extra arguments.
- DCHECK(!seen_constructor);
+ } else if (!seen_constructor) {
+ // At this point, we should see the constructor. However, we might have inlined it so we have
+ // to take care of both cases. We accept only the constructor with no extra arguments. This
+ // means that if we inline it, we have to check it is setting its field to a new array.
+ if (user->IsInvokeStaticOrDirect() &&
+ user->AsInvokeStaticOrDirect()->GetResolvedMethod() != nullptr &&
+ user->AsInvokeStaticOrDirect()->GetResolvedMethod()->IsConstructor() &&
+ user->AsInvokeStaticOrDirect()->GetNumberOfArguments() == 1u) {
+ constructor_inlined = false;
+ } else if (user->IsInstanceFieldSet() &&
+ user->AsInstanceFieldSet()->GetFieldType() == DataType::Type::kReference &&
+ user->AsInstanceFieldSet()->InputAt(0) == sb &&
+ user->AsInstanceFieldSet()->GetValue()->IsNewArray()) {
+ maybe_new_array = user->AsInstanceFieldSet()->GetValue();
+ constructor_inlined = true;
+ } else {
+ // We were expecting a constructor but we haven't seen it. Abort optimization.
+ return false;
+ }
DCHECK(!seen_constructor_fence);
seen_constructor = true;
} else if (user->IsConstructorFence()) {
@@ -2763,6 +2778,10 @@
// Accept only calls on the StringBuilder (which shall all be removed).
// TODO: Carve-out for const-string? Or rely on environment pruning (to be implemented)?
if (holder->InputCount() == 0 || holder->InputAt(0) != sb) {
+ // When inlining the constructor, we have a NewArray as an environment use.
+ if (constructor_inlined && holder == maybe_new_array) {
+ continue;
+ }
return false;
}
}
@@ -2796,6 +2815,18 @@
while (sb->HasNonEnvironmentUses()) {
block->RemoveInstruction(sb->GetUses().front().GetUser());
}
+ if (constructor_inlined) {
+ // We need to remove the inlined constructor instructions. That also removes all remaining
+ // environment uses.
+ DCHECK(sb->HasEnvironmentUses());
+ DCHECK(maybe_new_array != nullptr);
+ DCHECK(maybe_new_array->IsNewArray());
+ DCHECK(maybe_new_array->HasNonEnvironmentUses());
+ HInstruction* fence = maybe_new_array->GetUses().front().GetUser();
+ DCHECK(fence->IsConstructorFence());
+ block->RemoveInstruction(fence);
+ block->RemoveInstruction(maybe_new_array);
+ }
DCHECK(!sb->HasEnvironmentUses());
block->RemoveInstruction(sb);
return true;
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 06fb88e..6ef29bf 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -2286,6 +2286,9 @@
}
virtual bool NeedsEnvironment() const { return false; }
+ virtual bool NeedsBss() const {
+ return false;
+ }
uint32_t GetDexPc() const { return dex_pc_; }
@@ -4882,6 +4885,9 @@
}
bool IsClonable() const override { return true; }
+ bool NeedsBss() const override {
+ return GetMethodLoadKind() == MethodLoadKind::kBssEntry;
+ }
void SetDispatchInfo(DispatchInfo dispatch_info) {
bool had_current_method_input = HasCurrentMethodInput();
@@ -5167,6 +5173,9 @@
}
bool IsClonable() const override { return true; }
+ bool NeedsBss() const override {
+ return GetHiddenArgumentLoadKind() == MethodLoadKind::kBssEntry;
+ }
bool CanDoImplicitNullCheckOn(HInstruction* obj) const override {
// TODO: Add implicit null checks in intrinsics.
@@ -6813,6 +6822,12 @@
bool NeedsEnvironment() const override {
return CanCallRuntime();
}
+ bool NeedsBss() const override {
+ LoadKind load_kind = GetLoadKind();
+ return load_kind == LoadKind::kBssEntry ||
+ load_kind == LoadKind::kBssEntryPublic ||
+ load_kind == LoadKind::kBssEntryPackage;
+ }
void SetMustGenerateClinitCheck(bool generate_clinit_check) {
SetPackedFlag<kFlagGenerateClInitCheck>(generate_clinit_check);
@@ -7010,6 +7025,9 @@
}
bool IsClonable() const override { return true; }
+ bool NeedsBss() const override {
+ return GetLoadKind() == LoadKind::kBssEntry;
+ }
void SetLoadKind(LoadKind load_kind);
diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h
index 58d65bb..622fec3 100644
--- a/compiler/optimizing/optimizing_compiler_stats.h
+++ b/compiler/optimizing/optimizing_compiler_stats.h
@@ -81,6 +81,7 @@
kSimplifyThrowingInvoke,
kInstructionSunk,
kNotInlinedUnresolvedEntrypoint,
+ kNotInlinedBss,
kNotInlinedDexCache,
kNotInlinedStackMaps,
kNotInlinedEnvironmentBudget,
diff --git a/compiler/optimizing/stack_map_stream.cc b/compiler/optimizing/stack_map_stream.cc
index e52a3ce..964b48c 100644
--- a/compiler/optimizing/stack_map_stream.cc
+++ b/compiler/optimizing/stack_map_stream.cc
@@ -17,10 +17,14 @@
#include "stack_map_stream.h"
#include <memory>
+#include <vector>
#include "art_method-inl.h"
#include "base/stl_util.h"
+#include "class_linker.h"
+#include "dex/dex_file.h"
#include "dex/dex_file_types.h"
+#include "optimizing/nodes.h"
#include "optimizing/optimizing_compiler.h"
#include "runtime.h"
#include "scoped_thread_state_change-inl.h"
@@ -211,12 +215,26 @@
entry[InlineInfo::kArtMethodHi] = High32Bits(reinterpret_cast<uintptr_t>(method));
entry[InlineInfo::kArtMethodLo] = Low32Bits(reinterpret_cast<uintptr_t>(method));
} else {
- if (dex_pc != static_cast<uint32_t>(-1) && kIsDebugBuild) {
+ uint32_t bootclasspath_index = MethodInfo::kSameDexFile;
+ if (dex_pc != static_cast<uint32_t>(-1)) {
ScopedObjectAccess soa(Thread::Current());
- DCHECK(IsSameDexFile(*outer_dex_file, *method->GetDexFile()));
+ const DexFile* dex_file = method->GetDexFile();
+ if (method->GetDeclaringClass()->GetClassLoader() == nullptr) {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ const std::vector<const DexFile*>& boot_class_path = class_linker->GetBootClassPath();
+ auto it = std::find_if(
+ boot_class_path.begin(), boot_class_path.end(), [dex_file](const DexFile* df) {
+ return IsSameDexFile(*df, *dex_file);
+ });
+ DCHECK(it != boot_class_path.end());
+ bootclasspath_index = std::distance(boot_class_path.begin(), it);
+ } else {
+ DCHECK(IsSameDexFile(*outer_dex_file, *dex_file));
+ }
}
uint32_t dex_method_index = method->GetDexMethodIndex();
- entry[InlineInfo::kMethodInfoIndex] = method_infos_.Dedup({dex_method_index});
+ entry[InlineInfo::kMethodInfoIndex] =
+ method_infos_.Dedup({dex_method_index, bootclasspath_index});
}
current_inline_infos_.push_back(entry);
@@ -232,7 +250,18 @@
if (encode_art_method) {
CHECK_EQ(inline_info.GetArtMethod(), method);
} else {
- CHECK_EQ(code_info.GetMethodIndexOf(inline_info), method->GetDexMethodIndex());
+ MethodInfo method_info = code_info.GetMethodInfoOf(inline_info);
+ CHECK_EQ(method_info.GetMethodIndex(), method->GetDexMethodIndex());
+ if (inline_info.GetDexPc() != static_cast<uint32_t>(-1)) {
+ ScopedObjectAccess soa(Thread::Current());
+ if (method->GetDeclaringClass()->GetClassLoader() == nullptr) {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ const std::vector<const DexFile*>& boot_class_path = class_linker->GetBootClassPath();
+ DCHECK_LT(method_info.GetDexFileIndex(), boot_class_path.size());
+ CHECK(IsSameDexFile(*boot_class_path[method_info.GetDexFileIndex()],
+ *method->GetDexFile()));
+ }
+ }
}
});
}