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/code_generator.cc | 26 +++++------- compiler/optimizing/code_generator.h | 9 ++-- compiler/optimizing/code_generator_arm.cc | 18 +++++--- compiler/optimizing/code_generator_arm.h | 4 +- compiler/optimizing/code_generator_arm64.cc | 18 +++++--- compiler/optimizing/code_generator_arm64.h | 3 +- compiler/optimizing/code_generator_arm_vixl.cc | 21 ++++++---- compiler/optimizing/code_generator_arm_vixl.h | 3 +- compiler/optimizing/code_generator_mips.cc | 9 ++-- compiler/optimizing/code_generator_mips64.cc | 9 ++-- compiler/optimizing/code_generator_x86.cc | 17 +++++--- compiler/optimizing/code_generator_x86.h | 4 +- compiler/optimizing/code_generator_x86_64.cc | 19 +++++---- compiler/optimizing/code_generator_x86_64.h | 4 +- compiler/optimizing/nodes.cc | 25 +++++++---- compiler/optimizing/nodes.h | 58 +++++++------------------- compiler/optimizing/optimizing_compiler.cc | 2 +- compiler/optimizing/sharpening.cc | 27 ++++-------- 18 files changed, 141 insertions(+), 135 deletions(-) (limited to 'compiler/optimizing') diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index 402eeee65f..f00648f570 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -1378,28 +1378,21 @@ uint32_t CodeGenerator::GetReferenceDisableFlagOffset() const { void CodeGenerator::EmitJitRoots(uint8_t* code, Handle> roots, - const uint8_t* roots_data, - Handle outer_dex_cache) { + const uint8_t* roots_data) { DCHECK_EQ(static_cast(roots->GetLength()), GetNumberOfJitRoots()); - StackHandleScope<1> hs(Thread::Current()); - MutableHandle h_dex_cache(hs.NewHandle(nullptr)); ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); size_t index = 0; for (auto& entry : jit_string_roots_) { - const DexFile& entry_dex_file = *entry.first.dex_file; - // Avoid the expensive FindDexCache call by checking if the string is - // in the compiled method's dex file. - h_dex_cache.Assign(IsSameDexFile(*outer_dex_cache->GetDexFile(), entry_dex_file) - ? outer_dex_cache.Get() - : class_linker->FindDexCache(hs.Self(), entry_dex_file)); - mirror::String* string = class_linker->LookupString( - entry_dex_file, entry.first.string_index, h_dex_cache); - DCHECK(string != nullptr) << "JIT roots require strings to have been loaded"; + // Update the `roots` with the string, and replace the address temporarily + // stored to the index in the table. + uint64_t address = entry.second; + roots->Set(index, reinterpret_cast*>(address)->AsMirrorPtr()); + DCHECK(roots->Get(index) != nullptr); + entry.second = index; // Ensure the string is strongly interned. This is a requirement on how the JIT // handles strings. b/32995596 - class_linker->GetInternTable()->InternStrong(string); - roots->Set(index, string); - entry.second = index; + class_linker->GetInternTable()->InternStrong( + reinterpret_cast(roots->Get(index))); ++index; } for (auto& entry : jit_class_roots_) { @@ -1407,6 +1400,7 @@ void CodeGenerator::EmitJitRoots(uint8_t* code, // stored to the index in the table. uint64_t address = entry.second; roots->Set(index, reinterpret_cast*>(address)->AsMirrorPtr()); + DCHECK(roots->Get(index) != nullptr); entry.second = index; ++index; } diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h index 2e2c3c00af..6366b9838f 100644 --- a/compiler/optimizing/code_generator.h +++ b/compiler/optimizing/code_generator.h @@ -351,8 +351,7 @@ class CodeGenerator : public DeletableArenaObject { // Also emits literal patches. void EmitJitRoots(uint8_t* code, Handle> roots, - const uint8_t* roots_data, - Handle outer_dex_cache) + const uint8_t* roots_data) REQUIRES_SHARED(Locks::mutator_lock_); bool IsLeafMethod() const { @@ -713,9 +712,9 @@ class CodeGenerator : public DeletableArenaObject { const ArenaVector* block_order_; // Maps a StringReference (dex_file, string_index) to the index in the literal table. - // Entries are intially added with a 0 index, and `EmitJitRoots` will compute all the - // indices. - ArenaSafeMap jit_string_roots_; + // Entries are intially added with a pointer in the handle zone, and `EmitJitRoots` + // will compute all the indices. + ArenaSafeMap jit_string_roots_; // Maps a ClassReference (dex_file, type_index) to the index in the literal table. // Entries are intially added with a pointer in the handle zone, and `EmitJitRoots` diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 8a7f6d3a33..3009103ac7 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -5937,7 +5937,9 @@ void LocationsBuilderARM::VisitLoadString(HLoadString* load) { } } -void InstructionCodeGeneratorARM::VisitLoadString(HLoadString* load) { +// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not +// move. +void InstructionCodeGeneratorARM::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS { LocationSummary* locations = load->GetLocations(); Location out_loc = locations->Out(); Register out = out_loc.AsRegister(); @@ -5962,8 +5964,9 @@ void InstructionCodeGeneratorARM::VisitLoadString(HLoadString* load) { return; // No dex cache slow path. } case HLoadString::LoadKind::kBootImageAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); + uint32_t address = dchecked_integral_cast( + reinterpret_cast(load->GetString().Get())); + DCHECK_NE(address, 0u); __ LoadLiteral(out, codegen_->DeduplicateBootImageAddressLiteral(address)); return; // No dex cache slow path. } @@ -5987,7 +5990,8 @@ void InstructionCodeGeneratorARM::VisitLoadString(HLoadString* load) { } case HLoadString::LoadKind::kJitTableAddress: { __ LoadLiteral(out, codegen_->DeduplicateJitStringLiteral(load->GetDexFile(), - load->GetStringIndex())); + load->GetStringIndex(), + load->GetString())); // /* GcRoot */ out = *out GenerateGcRootFieldLoad(load, out_loc, out, /* offset */ 0, kCompilerReadBarrierOption); return; @@ -7317,8 +7321,10 @@ Literal* CodeGeneratorARM::DeduplicateBootImageAddressLiteral(uint32_t address) } Literal* CodeGeneratorARM::DeduplicateJitStringLiteral(const DexFile& dex_file, - dex::StringIndex string_index) { - jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), /* placeholder */ 0u); + dex::StringIndex string_index, + Handle handle) { + jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), + reinterpret_cast64(handle.GetReference())); return jit_string_patches_.GetOrCreate( StringReference(&dex_file, string_index), [this]() { return __ NewLiteral(/* placeholder */ 0u); }); diff --git a/compiler/optimizing/code_generator_arm.h b/compiler/optimizing/code_generator_arm.h index 6435851320..d5968e0764 100644 --- a/compiler/optimizing/code_generator_arm.h +++ b/compiler/optimizing/code_generator_arm.h @@ -489,7 +489,9 @@ class CodeGeneratorARM : public CodeGenerator { dex::StringIndex string_index); Literal* DeduplicateBootImageTypeLiteral(const DexFile& dex_file, dex::TypeIndex type_index); Literal* DeduplicateBootImageAddressLiteral(uint32_t address); - Literal* DeduplicateJitStringLiteral(const DexFile& dex_file, dex::StringIndex string_index); + Literal* DeduplicateJitStringLiteral(const DexFile& dex_file, + dex::StringIndex string_index, + Handle handle); Literal* DeduplicateJitClassLiteral(const DexFile& dex_file, dex::TypeIndex type_index, uint64_t address); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index 5c33fe1a7d..4b6a9bed61 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -4137,8 +4137,9 @@ vixl::aarch64::Literal* CodeGeneratorARM64::DeduplicateBootImageAddres } vixl::aarch64::Literal* CodeGeneratorARM64::DeduplicateJitStringLiteral( - const DexFile& dex_file, dex::StringIndex string_index) { - jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), /* placeholder */ 0u); + const DexFile& dex_file, dex::StringIndex string_index, Handle handle) { + jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), + reinterpret_cast64(handle.GetReference())); return jit_string_patches_.GetOrCreate( StringReference(&dex_file, string_index), [this]() { return __ CreateLiteralDestroyedWithPool(/* placeholder */ 0u); }); @@ -4527,7 +4528,9 @@ void LocationsBuilderARM64::VisitLoadString(HLoadString* load) { } } -void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) { +// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not +// move. +void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS { Register out = OutputRegister(load); Location out_loc = load->GetLocations()->Out(); @@ -4550,8 +4553,10 @@ void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) { return; // No dex cache slow path. } case HLoadString::LoadKind::kBootImageAddress: { - DCHECK(load->GetAddress() != 0u && IsUint<32>(load->GetAddress())); - __ Ldr(out.W(), codegen_->DeduplicateBootImageAddressLiteral(load->GetAddress())); + uint32_t address = dchecked_integral_cast( + reinterpret_cast(load->GetString().Get())); + DCHECK_NE(address, 0u); + __ Ldr(out.W(), codegen_->DeduplicateBootImageAddressLiteral(address)); return; // No dex cache slow path. } case HLoadString::LoadKind::kBssEntry: { @@ -4582,7 +4587,8 @@ void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) { } case HLoadString::LoadKind::kJitTableAddress: { __ Ldr(out, codegen_->DeduplicateJitStringLiteral(load->GetDexFile(), - load->GetStringIndex())); + load->GetStringIndex(), + load->GetString())); GenerateGcRootFieldLoad(load, out_loc, out.X(), diff --git a/compiler/optimizing/code_generator_arm64.h b/compiler/optimizing/code_generator_arm64.h index 8f33b6becf..d6a5f9d1fa 100644 --- a/compiler/optimizing/code_generator_arm64.h +++ b/compiler/optimizing/code_generator_arm64.h @@ -567,7 +567,8 @@ class CodeGeneratorARM64 : public CodeGenerator { dex::TypeIndex type_index); vixl::aarch64::Literal* DeduplicateBootImageAddressLiteral(uint64_t address); vixl::aarch64::Literal* DeduplicateJitStringLiteral(const DexFile& dex_file, - dex::StringIndex string_index); + dex::StringIndex string_index, + Handle handle); vixl::aarch64::Literal* DeduplicateJitClassLiteral(const DexFile& dex_file, dex::TypeIndex string_index, uint64_t address); diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index 00ad3e34b7..b1f6d599ab 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -6022,7 +6022,9 @@ void LocationsBuilderARMVIXL::VisitLoadString(HLoadString* load) { } } -void InstructionCodeGeneratorARMVIXL::VisitLoadString(HLoadString* load) { +// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not +// move. +void InstructionCodeGeneratorARMVIXL::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS { LocationSummary* locations = load->GetLocations(); Location out_loc = locations->Out(); vixl32::Register out = OutputRegister(load); @@ -6042,8 +6044,9 @@ void InstructionCodeGeneratorARMVIXL::VisitLoadString(HLoadString* load) { return; // No dex cache slow path. } case HLoadString::LoadKind::kBootImageAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); + uint32_t address = dchecked_integral_cast( + reinterpret_cast(load->GetString().Get())); + DCHECK_NE(address, 0u); __ Ldr(out, codegen_->DeduplicateBootImageAddressLiteral(address)); return; // No dex cache slow path. } @@ -6063,7 +6066,8 @@ void InstructionCodeGeneratorARMVIXL::VisitLoadString(HLoadString* load) { } case HLoadString::LoadKind::kJitTableAddress: { __ Ldr(out, codegen_->DeduplicateJitStringLiteral(load->GetDexFile(), - load->GetStringIndex())); + load->GetStringIndex(), + load->GetString())); // /* GcRoot */ out = *out GenerateGcRootFieldLoad(load, out_loc, out, /* offset */ 0, kCompilerReadBarrierOption); return; @@ -7444,9 +7448,12 @@ VIXLUInt32Literal* CodeGeneratorARMVIXL::DeduplicateDexCacheAddressLiteral(uint3 return DeduplicateUint32Literal(address, &uint32_literals_); } -VIXLUInt32Literal* CodeGeneratorARMVIXL::DeduplicateJitStringLiteral(const DexFile& dex_file, - dex::StringIndex string_index) { - jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), /* placeholder */ 0u); +VIXLUInt32Literal* CodeGeneratorARMVIXL::DeduplicateJitStringLiteral( + const DexFile& dex_file, + dex::StringIndex string_index, + Handle handle) { + jit_string_roots_.Overwrite(StringReference(&dex_file, string_index), + reinterpret_cast64(handle.GetReference())); return jit_string_patches_.GetOrCreate( StringReference(&dex_file, string_index), [this]() { diff --git a/compiler/optimizing/code_generator_arm_vixl.h b/compiler/optimizing/code_generator_arm_vixl.h index 297d63cefd..200a463c75 100644 --- a/compiler/optimizing/code_generator_arm_vixl.h +++ b/compiler/optimizing/code_generator_arm_vixl.h @@ -573,7 +573,8 @@ class CodeGeneratorARMVIXL : public CodeGenerator { VIXLUInt32Literal* DeduplicateBootImageAddressLiteral(uint32_t address); VIXLUInt32Literal* DeduplicateDexCacheAddressLiteral(uint32_t address); VIXLUInt32Literal* DeduplicateJitStringLiteral(const DexFile& dex_file, - dex::StringIndex string_index); + dex::StringIndex string_index, + Handle handle); VIXLUInt32Literal* DeduplicateJitClassLiteral(const DexFile& dex_file, dex::TypeIndex type_index, uint64_t address); diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 01e0dac33e..9af03e8153 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -5625,7 +5625,9 @@ void LocationsBuilderMIPS::VisitLoadString(HLoadString* load) { } } -void InstructionCodeGeneratorMIPS::VisitLoadString(HLoadString* load) { +// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not +// move. +void InstructionCodeGeneratorMIPS::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS { HLoadString::LoadKind load_kind = load->GetLoadKind(); LocationSummary* locations = load->GetLocations(); Location out_loc = locations->Out(); @@ -5660,8 +5662,9 @@ void InstructionCodeGeneratorMIPS::VisitLoadString(HLoadString* load) { return; // No dex cache slow path. } case HLoadString::LoadKind::kBootImageAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); + uint32_t address = dchecked_integral_cast( + reinterpret_cast(load->GetString().Get())); + DCHECK_NE(address, 0u); __ LoadLiteral(out, base_or_current_method_reg, codegen_->DeduplicateBootImageAddressLiteral(address)); diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 36690c0569..046d59cee7 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -3628,7 +3628,9 @@ void LocationsBuilderMIPS64::VisitLoadString(HLoadString* load) { } } -void InstructionCodeGeneratorMIPS64::VisitLoadString(HLoadString* load) { +// NO_THREAD_SAFETY_ANALYSIS as we manipulate handles whose internal object we know does not +// move. +void InstructionCodeGeneratorMIPS64::VisitLoadString(HLoadString* load) NO_THREAD_SAFETY_ANALYSIS { HLoadString::LoadKind load_kind = load->GetLoadKind(); LocationSummary* locations = load->GetLocations(); Location out_loc = locations->Out(); @@ -3650,8 +3652,9 @@ void InstructionCodeGeneratorMIPS64::VisitLoadString(HLoadString* load) { return; // No dex cache slow path. } case HLoadString::LoadKind::kBootImageAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); + uint32_t address = dchecked_integral_cast( + reinterpret_cast(load->GetString().Get())); + DCHECK_NE(address, 0u); __ LoadLiteral(out, kLoadUnsignedWord, codegen_->DeduplicateBootImageAddressLiteral(address)); diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 0abe85540c..f13b60aebf 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -6232,15 +6232,19 @@ void LocationsBuilderX86::VisitLoadString(HLoadString* load) { } Label* CodeGeneratorX86::NewJitRootStringPatch(const DexFile& dex_file, - dex::StringIndex dex_index) { - jit_string_roots_.Overwrite(StringReference(&dex_file, dex_index), /* placeholder */ 0u); + dex::StringIndex dex_index, + Handle handle) { + jit_string_roots_.Overwrite( + StringReference(&dex_file, dex_index), reinterpret_cast64(handle.GetReference())); // Add a patch entry and return the label. jit_string_patches_.emplace_back(dex_file, dex_index.index_); PatchInfo