From 68c981fad87720fae9c799b240141ce3c12cd5bf Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Fri, 26 Aug 2016 13:13:33 +0100 Subject: [PATCH] ARM/MIPS: Avoid dead dex cache arrays base for intrinsics. Test: Run ART test suite on host and Nexus 6. Change-Id: Ie2ad70f1e3f125eae5dad53a6384d405e0311505 --- compiler/optimizing/code_generator_arm.cc | 8 ++------ compiler/optimizing/dex_cache_array_fixups_arm.cc | 12 +++++++++--- compiler/optimizing/dex_cache_array_fixups_arm.h | 11 +++++++++-- compiler/optimizing/dex_cache_array_fixups_mips.cc | 4 +++- compiler/optimizing/intrinsics.h | 18 ++++++++++++++++++ compiler/optimizing/intrinsics_arm.cc | 5 +++++ compiler/optimizing/intrinsics_arm.h | 5 +---- compiler/optimizing/optimizing_compiler.cc | 5 +++-- compiler/optimizing/pc_relative_fixups_mips.cc | 19 ++----------------- compiler/optimizing/pc_relative_fixups_x86.cc | 18 +----------------- 10 files changed, 53 insertions(+), 52 deletions(-) diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index ba2df0709..4ea17a927 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -1857,9 +1857,7 @@ void LocationsBuilderARM::VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invok // art::PrepareForRegisterAllocation. DCHECK(!invoke->IsStaticWithExplicitClinitCheck()); - IntrinsicLocationsBuilderARM intrinsic(GetGraph()->GetArena(), - codegen_->GetAssembler(), - codegen_->GetInstructionSetFeatures()); + IntrinsicLocationsBuilderARM intrinsic(codegen_); if (intrinsic.TryDispatch(invoke)) { if (invoke->GetLocations()->CanCall() && invoke->HasPcRelativeDexCache()) { invoke->GetLocations()->SetInAt(invoke->GetSpecialInputIndex(), Location::Any()); @@ -1905,9 +1903,7 @@ void LocationsBuilderARM::HandleInvoke(HInvoke* invoke) { } void LocationsBuilderARM::VisitInvokeVirtual(HInvokeVirtual* invoke) { - IntrinsicLocationsBuilderARM intrinsic(GetGraph()->GetArena(), - codegen_->GetAssembler(), - codegen_->GetInstructionSetFeatures()); + IntrinsicLocationsBuilderARM intrinsic(codegen_); if (intrinsic.TryDispatch(invoke)) { return; } diff --git a/compiler/optimizing/dex_cache_array_fixups_arm.cc b/compiler/optimizing/dex_cache_array_fixups_arm.cc index 14c318e21..6ad9b07f1 100644 --- a/compiler/optimizing/dex_cache_array_fixups_arm.cc +++ b/compiler/optimizing/dex_cache_array_fixups_arm.cc @@ -17,6 +17,8 @@ #include "dex_cache_array_fixups_arm.h" #include "base/arena_containers.h" +#include "code_generator_arm.h" +#include "intrinsics_arm.h" #include "utils/dex_cache_arrays_layout-inl.h" namespace art { @@ -27,8 +29,9 @@ namespace arm { */ class DexCacheArrayFixupsVisitor : public HGraphVisitor { public: - explicit DexCacheArrayFixupsVisitor(HGraph* graph) + DexCacheArrayFixupsVisitor(HGraph* graph, CodeGenerator* codegen) : HGraphVisitor(graph), + codegen_(down_cast(codegen)), dex_cache_array_bases_(std::less(), // Attribute memory use to code generator. graph->GetArena()->Adapter(kArenaAllocCodeGenerator)) {} @@ -77,7 +80,8 @@ class DexCacheArrayFixupsVisitor : public HGraphVisitor { void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE { // If this is an invoke with PC-relative access to the dex cache methods array, // we need to add the dex cache arrays base as the special input. - if (invoke->HasPcRelativeDexCache()) { + if (invoke->HasPcRelativeDexCache() && + !IsCallFreeIntrinsic(invoke, codegen_)) { // Initialize base for target method dex file if needed. MethodReference target_method = invoke->GetTargetMethod(); HArmDexCacheArraysBase* base = GetOrCreateDexCacheArrayBase(*target_method.dex_file); @@ -107,6 +111,8 @@ class DexCacheArrayFixupsVisitor : public HGraphVisitor { return base; } + CodeGeneratorARM* codegen_; + using DexCacheArraysBaseMap = ArenaSafeMap>; DexCacheArraysBaseMap dex_cache_array_bases_; @@ -118,7 +124,7 @@ void DexCacheArrayFixups::Run() { // that can be live-in at the irreducible loop header. return; } - DexCacheArrayFixupsVisitor visitor(graph_); + DexCacheArrayFixupsVisitor visitor(graph_, codegen_); visitor.VisitInsertionOrder(); visitor.MoveBasesIfNeeded(); } diff --git a/compiler/optimizing/dex_cache_array_fixups_arm.h b/compiler/optimizing/dex_cache_array_fixups_arm.h index 9142e29ef..9d67a319b 100644 --- a/compiler/optimizing/dex_cache_array_fixups_arm.h +++ b/compiler/optimizing/dex_cache_array_fixups_arm.h @@ -21,16 +21,23 @@ #include "optimization.h" namespace art { + +class CodeGenerator; + namespace arm { class DexCacheArrayFixups : public HOptimization { public: - DexCacheArrayFixups(HGraph* graph, OptimizingCompilerStats* stats) - : HOptimization(graph, kDexCacheArrayFixupsArmPassName, stats) {} + DexCacheArrayFixups(HGraph* graph, CodeGenerator* codegen, OptimizingCompilerStats* stats) + : HOptimization(graph, kDexCacheArrayFixupsArmPassName, stats), + codegen_(codegen) {} static constexpr const char* kDexCacheArrayFixupsArmPassName = "dex_cache_array_fixups_arm"; void Run() OVERRIDE; + + private: + CodeGenerator* codegen_; }; } // namespace arm diff --git a/compiler/optimizing/dex_cache_array_fixups_mips.cc b/compiler/optimizing/dex_cache_array_fixups_mips.cc index 19bab08eb..300284d04 100644 --- a/compiler/optimizing/dex_cache_array_fixups_mips.cc +++ b/compiler/optimizing/dex_cache_array_fixups_mips.cc @@ -18,6 +18,7 @@ #include "dex_cache_array_fixups_mips.h" #include "base/arena_containers.h" +#include "intrinsics_mips.h" #include "utils/dex_cache_arrays_layout-inl.h" namespace art { @@ -85,7 +86,8 @@ class DexCacheArrayFixupsVisitor : public HGraphVisitor { void VisitInvokeStaticOrDirect(HInvokeStaticOrDirect* invoke) OVERRIDE { // If this is an invoke with PC-relative access to the dex cache methods array, // we need to add the dex cache arrays base as the special input. - if (invoke->HasPcRelativeDexCache()) { + if (invoke->HasPcRelativeDexCache() && + !IsCallFreeIntrinsic(invoke, codegen_)) { // Initialize base for target method dex file if needed. MethodReference target_method = invoke->GetTargetMethod(); HMipsDexCacheArraysBase* base = GetOrCreateDexCacheArrayBase(*target_method.dex_file); diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h index 1a8eb5885..62f731d03 100644 --- a/compiler/optimizing/intrinsics.h +++ b/compiler/optimizing/intrinsics.h @@ -243,6 +243,24 @@ UNREACHABLE_INTRINSIC(Arch, UnsafeLoadFence) \ UNREACHABLE_INTRINSIC(Arch, UnsafeStoreFence) \ UNREACHABLE_INTRINSIC(Arch, UnsafeFullFence) +template +bool IsCallFreeIntrinsic(HInvoke* invoke, Codegenerator* codegen) { + if (invoke->GetIntrinsic() != Intrinsics::kNone) { + // This invoke may have intrinsic code generation defined. However, we must + // now also determine if this code generation is truly there and call-free + // (not unimplemented, no bail on instruction features, or call on slow path). + // This is done by actually calling the locations builder on the instruction + // and clearing out the locations once result is known. We assume this + // call only has creating locations as side effects! + // TODO: Avoid wasting Arena memory. + IntrinsicLocationsBuilder builder(codegen); + bool success = builder.TryDispatch(invoke) && !invoke->GetLocations()->CanCall(); + invoke->SetLocations(nullptr); + return success; + } + return false; +} + } // namespace art #endif // ART_COMPILER_OPTIMIZING_INTRINSICS_H_ diff --git a/compiler/optimizing/intrinsics_arm.cc b/compiler/optimizing/intrinsics_arm.cc index e1a6454b5..df7cb85a4 100644 --- a/compiler/optimizing/intrinsics_arm.cc +++ b/compiler/optimizing/intrinsics_arm.cc @@ -127,6 +127,11 @@ class ReadBarrierSystemArrayCopySlowPathARM : public SlowPathCode { #undef __ +IntrinsicLocationsBuilderARM::IntrinsicLocationsBuilderARM(CodeGeneratorARM* codegen) + : arena_(codegen->GetGraph()->GetArena()), + assembler_(codegen->GetAssembler()), + features_(codegen->GetInstructionSetFeatures()) {} + bool IntrinsicLocationsBuilderARM::TryDispatch(HInvoke* invoke) { Dispatch(invoke); LocationSummary* res = invoke->GetLocations(); diff --git a/compiler/optimizing/intrinsics_arm.h b/compiler/optimizing/intrinsics_arm.h index e01b6fffb..c67170061 100644 --- a/compiler/optimizing/intrinsics_arm.h +++ b/compiler/optimizing/intrinsics_arm.h @@ -33,10 +33,7 @@ class CodeGeneratorARM; class IntrinsicLocationsBuilderARM FINAL : public IntrinsicVisitor { public: - IntrinsicLocationsBuilderARM(ArenaAllocator* arena, - ArmAssembler* assembler, - const ArmInstructionSetFeatures& features) - : arena_(arena), assembler_(assembler), features_(features) {} + explicit IntrinsicLocationsBuilderARM(CodeGeneratorARM* codegen); // Define visitor methods. diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index f7c325ed9..6e98b4d54 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -490,7 +490,7 @@ static HOptimization* BuildOptimization( return new (arena) SideEffectsAnalysis(graph); #ifdef ART_ENABLE_CODEGEN_arm } else if (opt_name == arm::DexCacheArrayFixups::kDexCacheArrayFixupsArmPassName) { - return new (arena) arm::DexCacheArrayFixups(graph, stats); + return new (arena) arm::DexCacheArrayFixups(graph, codegen, stats); } else if (opt_name == arm::InstructionSimplifierArm::kInstructionSimplifierArmPassName) { return new (arena) arm::InstructionSimplifierArm(graph, stats); #endif @@ -604,7 +604,8 @@ void OptimizingCompiler::RunArchOptimizations(InstructionSet instruction_set, #ifdef ART_ENABLE_CODEGEN_arm case kThumb2: case kArm: { - arm::DexCacheArrayFixups* fixups = new (arena) arm::DexCacheArrayFixups(graph, stats); + arm::DexCacheArrayFixups* fixups = + new (arena) arm::DexCacheArrayFixups(graph, codegen, stats); arm::InstructionSimplifierArm* simplifier = new (arena) arm::InstructionSimplifierArm(graph, stats); SideEffectsAnalysis* side_effects = new (arena) SideEffectsAnalysis(graph); diff --git a/compiler/optimizing/pc_relative_fixups_mips.cc b/compiler/optimizing/pc_relative_fixups_mips.cc index c6acc4558..c6d297df4 100644 --- a/compiler/optimizing/pc_relative_fixups_mips.cc +++ b/compiler/optimizing/pc_relative_fixups_mips.cc @@ -115,7 +115,8 @@ class PCRelativeHandlerVisitor : public HGraphVisitor { return; } - if (has_extra_input && !WillHaveCallFreeIntrinsicsCodeGen(invoke)) { + if (has_extra_input && + !IsCallFreeIntrinsic(invoke, codegen_)) { InitializePCRelativeBasePointer(); // Add the extra parameter base_. invoke_static_or_direct->AddSpecialInput(base_); @@ -123,22 +124,6 @@ class PCRelativeHandlerVisitor : public HGraphVisitor { } } - bool WillHaveCallFreeIntrinsicsCodeGen(HInvoke* invoke) { - if (invoke->GetIntrinsic() != Intrinsics::kNone) { - // This invoke may have intrinsic code generation defined. However, we must - // now also determine if this code generation is truly there and call-free - // (not unimplemented, no bail on instruction features, or call on slow path). - // This is done by actually calling the locations builder on the instruction - // and clearing out the locations once result is known. We assume this - // call only has creating locations as side effects! - IntrinsicLocationsBuilderMIPS builder(codegen_); - bool success = builder.TryDispatch(invoke) && !invoke->GetLocations()->CanCall(); - invoke->SetLocations(nullptr); - return success; - } - return false; - } - CodeGeneratorMIPS* codegen_; // The generated HMipsComputeBaseMethodAddress in the entry block needed as an diff --git a/compiler/optimizing/pc_relative_fixups_x86.cc b/compiler/optimizing/pc_relative_fixups_x86.cc index ad0921d7e..75587af7a 100644 --- a/compiler/optimizing/pc_relative_fixups_x86.cc +++ b/compiler/optimizing/pc_relative_fixups_x86.cc @@ -203,7 +203,7 @@ class PCRelativeHandlerVisitor : public HGraphVisitor { bool base_added = false; if (invoke_static_or_direct != nullptr && invoke_static_or_direct->HasPcRelativeDexCache() && - !WillHaveCallFreeIntrinsicsCodeGen(invoke)) { + !IsCallFreeIntrinsic(invoke, codegen_)) { InitializePCRelativeBasePointer(); // Add the extra parameter base_. invoke_static_or_direct->AddSpecialInput(base_); @@ -240,22 +240,6 @@ class PCRelativeHandlerVisitor : public HGraphVisitor { } } - bool WillHaveCallFreeIntrinsicsCodeGen(HInvoke* invoke) { - if (invoke->GetIntrinsic() != Intrinsics::kNone) { - // This invoke may have intrinsic code generation defined. However, we must - // now also determine if this code generation is truly there and call-free - // (not unimplemented, no bail on instruction features, or call on slow path). - // This is done by actually calling the locations builder on the instruction - // and clearing out the locations once result is known. We assume this - // call only has creating locations as side effects! - IntrinsicLocationsBuilderX86 builder(codegen_); - bool success = builder.TryDispatch(invoke) && !invoke->GetLocations()->CanCall(); - invoke->SetLocations(nullptr); - return success; - } - return false; - } - CodeGeneratorX86* codegen_; // The generated HX86ComputeBaseMethodAddress in the entry block needed as an -- 2.11.0