From f0acfe7a812a332122011832074142718c278dae Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Mon, 9 Jan 2017 20:54:52 +0000 Subject: Keep resolved String in HLoadString. For the following reasons: - Avoids needing to do a lookup again in CodeGenerator::EmitJitRoots. - Fixes races where we the string was GC'ed before CodeGenerator::EmitJitRoots. - Makes it possible to do GVN on the same string but defined in different dex files. Test: test-art-host, test-art-target Change-Id: If2b5d3079f7555427b1b96ab04546b3373fcf921 --- compiler/optimizing/nodes.cc | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'compiler/optimizing/nodes.cc') diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index a599c2aa84..d45fa11534 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -2498,6 +2498,17 @@ std::ostream& operator<<(std::ostream& os, HLoadClass::LoadKind rhs) { } } +// Helper for InstructionDataEquals to fetch the mirror String out +// from a kJitTableAddress LoadString kind. +// NO_THREAD_SAFETY_ANALYSIS because even though we're accessing +// mirrors, they are stored in a variable size handle scope which is always +// visited during a pause. Also, the only caller of this helper +// only uses the mirror for pointer comparison. +static inline mirror::String* AsMirrorInternal(Handle handle) + NO_THREAD_SAFETY_ANALYSIS { + return handle.Get(); +} + bool HLoadString::InstructionDataEquals(const HInstruction* other) const { const HLoadString* other_load_string = other->AsLoadString(); // TODO: To allow GVN for HLoadString from different dex files, we should compare the strings @@ -2506,16 +2517,16 @@ bool HLoadString::InstructionDataEquals(const HInstruction* other) const { GetPackedFields() != other_load_string->GetPackedFields()) { return false; } - LoadKind load_kind = GetLoadKind(); - if (HasAddress(load_kind)) { - return GetAddress() == other_load_string->GetAddress(); - } else { - DCHECK(HasStringReference(load_kind)) << load_kind; - return IsSameDexFile(GetDexFile(), other_load_string->GetDexFile()); + switch (GetLoadKind()) { + case LoadKind::kBootImageAddress: + case LoadKind::kJitTableAddress: + return AsMirrorInternal(GetString()) == AsMirrorInternal(other_load_string->GetString()); + default: + return IsSameDexFile(GetDexFile(), other_load_string->GetDexFile()); } } -void HLoadString::SetLoadKindInternal(LoadKind load_kind) { +void HLoadString::SetLoadKind(LoadKind load_kind) { // Once sharpened, the load kind should not be changed again. DCHECK_EQ(GetLoadKind(), LoadKind::kDexCacheViaMethod); SetPackedField(load_kind); -- cgit v1.2.3-59-g8ed1b