Revert "Inline across dex files for bootclaspath's methods"
This reverts commit d690f8ae8f8e2675bc52089a83ac18c749f8e6d2.
Reason for revert: Test breakage related to VarHandles e.g.
https://ci.chromium.org/ui/p/art/builders/ci/angler-armv7-ndebug/2409/overview
Change-Id: I0d3537cd0907866c628478fad1d77aff4ee65113
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 0e4f9ef..3abbbae 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1696,26 +1696,18 @@
return (object != hint.Get()) ? graph->GetHandleCache()->NewHandle(object) : hint;
}
-static bool CanEncodeInlinedMethodInStackMap(const DexFile& outer_dex_file,
- ArtMethod* callee,
- bool* out_needs_bss_check)
+static bool CanEncodeInlinedMethodInStackMap(const DexFile& caller_dex_file, ArtMethod* callee)
REQUIRES_SHARED(Locks::mutator_lock_) {
if (!Runtime::Current()->IsAotCompiler()) {
// JIT can always encode methods in stack maps.
return true;
}
- if (IsSameDexFile(outer_dex_file, *callee->GetDexFile())) {
+ if (IsSameDexFile(caller_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;
}
@@ -1825,11 +1817,6 @@
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()) {
@@ -1864,22 +1851,24 @@
return false;
}
HInstruction* current = instr_it.Current();
- 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() &&
+ (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 (!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->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 (current->IsUnresolvedStaticFieldGet() ||
@@ -1893,21 +1882,6 @@
<< " 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 3f1f3b0..91b2e8b 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2645,15 +2645,12 @@
// 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`.
@@ -2734,25 +2731,13 @@
format = (format << StringBuilderAppend::kBitsPerArg) | static_cast<uint32_t>(arg);
args[num_args] = as_invoke_virtual->InputAt(1u);
++num_args;
- } 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;
- }
+ } 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);
DCHECK(!seen_constructor_fence);
seen_constructor = true;
} else if (user->IsConstructorFence()) {
@@ -2778,10 +2763,6 @@
// 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;
}
}
@@ -2815,18 +2796,6 @@
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 6ef29bf..06fb88e 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -2286,9 +2286,6 @@
}
virtual bool NeedsEnvironment() const { return false; }
- virtual bool NeedsBss() const {
- return false;
- }
uint32_t GetDexPc() const { return dex_pc_; }
@@ -4885,9 +4882,6 @@
}
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();
@@ -5173,9 +5167,6 @@
}
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.
@@ -6822,12 +6813,6 @@
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);
@@ -7025,9 +7010,6 @@
}
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 622fec3..58d65bb 100644
--- a/compiler/optimizing/optimizing_compiler_stats.h
+++ b/compiler/optimizing/optimizing_compiler_stats.h
@@ -81,7 +81,6 @@
kSimplifyThrowingInvoke,
kInstructionSunk,
kNotInlinedUnresolvedEntrypoint,
- kNotInlinedBss,
kNotInlinedDexCache,
kNotInlinedStackMaps,
kNotInlinedEnvironmentBudget,
diff --git a/compiler/optimizing/stack_map_stream.cc b/compiler/optimizing/stack_map_stream.cc
index 964b48c..e52a3ce 100644
--- a/compiler/optimizing/stack_map_stream.cc
+++ b/compiler/optimizing/stack_map_stream.cc
@@ -17,14 +17,10 @@
#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"
@@ -215,26 +211,12 @@
entry[InlineInfo::kArtMethodHi] = High32Bits(reinterpret_cast<uintptr_t>(method));
entry[InlineInfo::kArtMethodLo] = Low32Bits(reinterpret_cast<uintptr_t>(method));
} else {
- uint32_t bootclasspath_index = MethodInfo::kSameDexFile;
- if (dex_pc != static_cast<uint32_t>(-1)) {
+ if (dex_pc != static_cast<uint32_t>(-1) && kIsDebugBuild) {
ScopedObjectAccess soa(Thread::Current());
- 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));
- }
+ DCHECK(IsSameDexFile(*outer_dex_file, *method->GetDexFile()));
}
uint32_t dex_method_index = method->GetDexMethodIndex();
- entry[InlineInfo::kMethodInfoIndex] =
- method_infos_.Dedup({dex_method_index, bootclasspath_index});
+ entry[InlineInfo::kMethodInfoIndex] = method_infos_.Dedup({dex_method_index});
}
current_inline_infos_.push_back(entry);
@@ -250,18 +232,7 @@
if (encode_art_method) {
CHECK_EQ(inline_info.GetArtMethod(), method);
} else {
- 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()));
- }
- }
+ CHECK_EQ(code_info.GetMethodIndexOf(inline_info), method->GetDexMethodIndex());
}
});
}
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index 5b931a1..84299d5 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -80,20 +80,10 @@
for (InlineInfo inline_info : inline_infos) {
DCHECK(!inline_info.EncodesArtMethod());
DCHECK_NE(inline_info.GetDexPc(), static_cast<uint32_t>(-1));
- MethodInfo method_info = code_info.GetMethodInfoOf(inline_info);
- uint32_t method_index = method_info.GetMethodIndex();
- ArtMethod* inlined_method;
- if (method_info.HasDexFileIndex()) {
- const DexFile* dex_file = class_linker->GetBootClassPath()[method_info.GetDexFileIndex()];
- ObjPtr<mirror::DexCache> dex_cache = class_linker->FindDexCache(Thread::Current(), *dex_file);
- // The class loader is always nullptr for this case so we can simplify the call.
- DCHECK_EQ(dex_cache->GetClassLoader(), nullptr);
- inlined_method = class_linker->LookupResolvedMethod(method_index, dex_cache, nullptr);
- } else {
- inlined_method = class_linker->LookupResolvedMethod(
- method_index, outer_method->GetDexCache(), outer_method->GetClassLoader());
- }
-
+ uint32_t method_index = code_info.GetMethodIndexOf(inline_info);
+ ArtMethod* inlined_method = class_linker->LookupResolvedMethod(method_index,
+ method->GetDexCache(),
+ method->GetClassLoader());
if (UNLIKELY(inlined_method == nullptr)) {
LOG(FATAL) << "Could not find an inlined method from an .oat file: "
<< method->GetDexFile()->PrettyMethod(method_index) << " . "
@@ -101,8 +91,7 @@
UNREACHABLE();
}
DCHECK(!inlined_method->IsRuntimeMethod());
- if (UNLIKELY(inlined_method->GetDexFile() != method->GetDexFile() &&
- !method_info.HasDexFileIndex())) {
+ if (UNLIKELY(inlined_method->GetDexFile() != method->GetDexFile())) {
// TODO: We could permit inlining within a multi-dex oat file and the boot image,
// even going back from boot image methods to the same oat file. However, this is
// not currently implemented in the compiler. Therefore crossing dex file boundary
@@ -110,14 +99,13 @@
bool target_sdk_at_least_p =
IsSdkVersionSetAndAtLeast(Runtime::Current()->GetTargetSdkVersion(), SdkVersion::kP);
LOG(target_sdk_at_least_p ? FATAL : WARNING)
- << "Inlined method resolution crossed dex file boundary: from " << method->PrettyMethod()
+ << "Inlined method resolution crossed dex file boundary: from "
+ << method->PrettyMethod()
<< " in " << method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(method->GetDexFile()) << " to "
- << inlined_method->PrettyMethod() << " in " << inlined_method->GetDexFile()->GetLocation()
- << "/" << static_cast<const void*>(inlined_method->GetDexFile()) << ". "
- << "The outermost method in the chain is: " << outer_method->PrettyMethod() << " in "
- << outer_method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(outer_method->GetDexFile())
+ << static_cast<const void*>(method->GetDexFile())
+ << " to " << inlined_method->PrettyMethod()
+ << " in " << inlined_method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(inlined_method->GetDexFile()) << ". "
<< "This must be due to duplicate classes or playing wrongly with class loaders. "
<< "The runtime is in an unsafe state.";
}
diff --git a/runtime/oat.h b/runtime/oat.h
index 6a20cc1..95eb0e1 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -32,8 +32,8 @@
class PACKED(4) OatHeader {
public:
static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } };
- // Last oat version changed reason: Inlining across dex files for bootclasspath methods.
- static constexpr std::array<uint8_t, 4> kOatVersion{ {'2', '0', '5', '\0'} };
+ // Last oat version changed reason: Inline IRT frame push/pop into JNI stubs.
+ static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '0', '3', '\0' } };
static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
static constexpr const char* kDebuggableKey = "debuggable";
diff --git a/runtime/stack_map.cc b/runtime/stack_map.cc
index 939c4b3..c160e2b 100644
--- a/runtime/stack_map.cc
+++ b/runtime/stack_map.cc
@@ -347,12 +347,9 @@
ScopedObjectAccess soa(Thread::Current());
vios->Stream() << ", method=" << GetArtMethod()->PrettyMethod();
} else {
- MethodInfo method_info = code_info.GetMethodInfoOf(*this);
- vios->Stream() << std::dec << ", method_index=" << method_info.GetMethodIndex();
- if (method_info.HasDexFileIndex()) {
- vios->Stream() << std::dec
- << ", boot_class_path_dex_file_index=" << method_info.GetDexFileIndex();
- }
+ vios->Stream()
+ << std::dec
+ << ", method_index=" << code_info.GetMethodIndexOf(*this);
}
vios->Stream() << ")\n";
code_info.GetInlineDexRegisterMapOf(stack_map, *this).Dump(vios);
diff --git a/runtime/stack_map.h b/runtime/stack_map.h
index 7148957..103402b 100644
--- a/runtime/stack_map.h
+++ b/runtime/stack_map.h
@@ -262,13 +262,10 @@
// Method indices are not very dedup friendly.
// Separating them greatly improves dedup efficiency of the other tables.
-class MethodInfo : public BitTableAccessor<2> {
+class MethodInfo : public BitTableAccessor<1> {
public:
BIT_TABLE_HEADER(MethodInfo)
BIT_TABLE_COLUMN(0, MethodIndex)
- BIT_TABLE_COLUMN(1, DexFileIndex)
-
- static constexpr uint32_t kSameDexFile = -1;
};
/**
@@ -363,12 +360,8 @@
return stack_maps_.NumRows();
}
- MethodInfo GetMethodInfoOf(InlineInfo inline_info) const {
- return method_infos_.GetRow(inline_info.GetMethodInfoIndex());
- }
-
uint32_t GetMethodIndexOf(InlineInfo inline_info) const {
- return GetMethodInfoOf(inline_info).GetMethodIndex();
+ return method_infos_.GetRow(inline_info.GetMethodInfoIndex()).GetMethodIndex();
}
ALWAYS_INLINE DexRegisterMap GetDexRegisterMapOf(StackMap stack_map) const {
diff --git a/test/580-checker-string-fact-intrinsics/src-art/Main.java b/test/580-checker-string-fact-intrinsics/src-art/Main.java
index 34af579..d0750f9 100644
--- a/test/580-checker-string-fact-intrinsics/src-art/Main.java
+++ b/test/580-checker-string-fact-intrinsics/src-art/Main.java
@@ -40,14 +40,16 @@
// java.lang.StringFactory.newStringFromChars(char[] data)
//
// which contains a call to the former (non-public) native method.
- // After the inliner runs, we can see the inlined call and check
- // that the compiler intrinsifies it.
+ // However, this call will not be inlined (because it is a method in
+ // another Dex file and which contains a call, which needs an
+ // environment), so we cannot use Checker here to ensure the native
+ // call was intrinsified either.
/// CHECK-START: void Main.testNewStringFromChars() builder (after)
/// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None
/// CHECK-START: void Main.testNewStringFromChars() inliner (after)
- /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:StringNewStringFromChars
+ /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None
public static void testNewStringFromChars() {
char[] chars = { 'b', 'a', 'r' };
