diff options
| -rw-r--r-- | compiler/dex/gvn_dead_code_elimination.cc | 5 | ||||
| -rw-r--r-- | compiler/dex/gvn_dead_code_elimination_test.cc | 85 | ||||
| -rw-r--r-- | compiler/driver/compiler_driver-inl.h | 25 | ||||
| -rw-r--r-- | compiler/driver/compiler_driver.cc | 21 | ||||
| -rw-r--r-- | compiler/driver/compiler_driver.h | 10 | ||||
| -rw-r--r-- | compiler/optimizing/dead_code_elimination.cc | 4 | ||||
| -rw-r--r-- | compiler/optimizing/nodes.cc | 5 | ||||
| -rw-r--r-- | compiler/optimizing/nodes.h | 1 | ||||
| -rw-r--r-- | runtime/oat.h | 2 | ||||
| -rw-r--r-- | test/Android.run-test.mk | 3 |
10 files changed, 148 insertions, 13 deletions
diff --git a/compiler/dex/gvn_dead_code_elimination.cc b/compiler/dex/gvn_dead_code_elimination.cc index 6d8a7dab2b..b1f5d870d4 100644 --- a/compiler/dex/gvn_dead_code_elimination.cc +++ b/compiler/dex/gvn_dead_code_elimination.cc @@ -1003,7 +1003,6 @@ bool GvnDeadCodeElimination::BackwardPassTryToKillLastMIR() { vreg_chains_.GetMIRData(kill_heads_[v_reg])->PrevChange(v_reg)); } } - unused_vregs_->Union(vregs_to_kill_); for (auto it = changes_to_kill_.rbegin(), end = changes_to_kill_.rend(); it != end; ++it) { MIRData* data = vreg_chains_.GetMIRData(*it); DCHECK(!data->must_keep); @@ -1012,6 +1011,10 @@ bool GvnDeadCodeElimination::BackwardPassTryToKillLastMIR() { KillMIR(data); } + // Each dependent register not in vregs_to_kill_ is either already marked unused or + // it's one word of a wide register where the other word has been overwritten. + unused_vregs_->UnionIfNotIn(dependent_vregs_, vregs_to_kill_); + vreg_chains_.RemoveTrailingNops(); return true; } diff --git a/compiler/dex/gvn_dead_code_elimination_test.cc b/compiler/dex/gvn_dead_code_elimination_test.cc index de591d0edb..461c844a60 100644 --- a/compiler/dex/gvn_dead_code_elimination_test.cc +++ b/compiler/dex/gvn_dead_code_elimination_test.cc @@ -137,6 +137,8 @@ class GvnDeadCodeEliminationTest : public testing::Test { { bb, opcode, 0u, 0u, 1, { src1 }, 1, { result } } #define DEF_BINOP(bb, opcode, result, src1, src2) \ { bb, opcode, 0u, 0u, 2, { src1, src2 }, 1, { result } } +#define DEF_BINOP_WIDE(bb, opcode, result, src1, src2) \ + { bb, opcode, 0u, 0u, 4, { src1, src1 + 1, src2, src2 + 1 }, 2, { result, result + 1 } } void DoPrepareIFields(const IFieldDef* defs, size_t count) { cu_.mir_graph->ifield_lowering_infos_.clear(); @@ -1936,7 +1938,7 @@ TEST_F(GvnDeadCodeEliminationTestSimple, MixedOverlaps1) { DEF_CONST(3, Instruction::CONST, 0u, 1000u), DEF_MOVE(3, Instruction::MOVE, 1u, 0u), DEF_CONST(3, Instruction::CONST, 2u, 2000u), - { 3, Instruction::INT_TO_LONG, 0, 0u, 1, { 2u }, 2, { 3u, 4u} }, + { 3, Instruction::INT_TO_LONG, 0, 0u, 1, { 2u }, 2, { 3u, 4u } }, DEF_MOVE_WIDE(3, Instruction::MOVE_WIDE, 5u, 3u), DEF_CONST(3, Instruction::CONST, 7u, 3000u), DEF_CONST(3, Instruction::CONST, 8u, 4000u), @@ -1983,4 +1985,85 @@ TEST_F(GvnDeadCodeEliminationTestSimple, MixedOverlaps1) { EXPECT_EQ(0u, int_to_long->dalvikInsn.vB); } +TEST_F(GvnDeadCodeEliminationTestSimple, UnusedRegs1) { + static const MIRDef mirs[] = { + DEF_CONST(3, Instruction::CONST, 0u, 1000u), + DEF_CONST(3, Instruction::CONST, 1u, 2000u), + DEF_BINOP(3, Instruction::ADD_INT, 2u, 1u, 0u), + DEF_CONST(3, Instruction::CONST, 3u, 1000u), // NOT killed (b/21702651). + DEF_BINOP(3, Instruction::ADD_INT, 4u, 1u, 3u), // Killed (RecordPass) + DEF_CONST(3, Instruction::CONST, 5u, 2000u), // Killed with 9u (BackwardPass) + DEF_BINOP(3, Instruction::ADD_INT, 6u, 5u, 0u), // Killed (RecordPass) + DEF_CONST(3, Instruction::CONST, 7u, 4000u), + DEF_MOVE(3, Instruction::MOVE, 8u, 0u), // Killed with 6u (BackwardPass) + }; + + static const int32_t sreg_to_vreg_map[] = { 1, 2, 3, 0, 3, 0, 3, 4, 0 }; + PrepareSRegToVRegMap(sreg_to_vreg_map); + + PrepareMIRs(mirs); + PerformGVN_DCE(); + + ASSERT_EQ(arraysize(mirs), value_names_.size()); + static const size_t diff_indexes[] = { 0, 1, 2, 7 }; + ExpectValueNamesNE(diff_indexes); + EXPECT_EQ(value_names_[0], value_names_[3]); + EXPECT_EQ(value_names_[2], value_names_[4]); + EXPECT_EQ(value_names_[1], value_names_[5]); + EXPECT_EQ(value_names_[2], value_names_[6]); + EXPECT_EQ(value_names_[0], value_names_[8]); + + static const bool eliminated[] = { + false, false, false, false, true, true, true, false, true, + }; + static_assert(arraysize(eliminated) == arraysize(mirs), "array size mismatch"); + for (size_t i = 0; i != arraysize(eliminated); ++i) { + bool actually_eliminated = (static_cast<int>(mirs_[i].dalvikInsn.opcode) == kMirOpNop); + EXPECT_EQ(eliminated[i], actually_eliminated) << i; + } +} + +TEST_F(GvnDeadCodeEliminationTestSimple, UnusedRegs2) { + static const MIRDef mirs[] = { + DEF_CONST(3, Instruction::CONST, 0u, 1000u), + DEF_CONST(3, Instruction::CONST, 1u, 2000u), + DEF_BINOP(3, Instruction::ADD_INT, 2u, 1u, 0u), + DEF_CONST(3, Instruction::CONST, 3u, 1000u), // Killed (BackwardPass; b/21702651) + DEF_BINOP(3, Instruction::ADD_INT, 4u, 1u, 3u), // Killed (RecordPass) + DEF_CONST_WIDE(3, Instruction::CONST_WIDE, 5u, 4000u), + { 3, Instruction::LONG_TO_INT, 0, 0u, 2, { 5u, 6u }, 1, { 7u } }, + DEF_BINOP(3, Instruction::ADD_INT, 8u, 7u, 0u), + DEF_CONST_WIDE(3, Instruction::CONST_WIDE, 9u, 4000u), // Killed with 12u (BackwardPass) + DEF_CONST(3, Instruction::CONST, 11u, 6000u), + { 3, Instruction::LONG_TO_INT, 0, 0u, 2, { 9u, 10u }, 1, { 12u } }, // Killed with 9u (BP) + }; + + static const int32_t sreg_to_vreg_map[] = { + 2, 3, 4, 1, 4, 5, 6 /* high word */, 0, 7, 0, 1 /* high word */, 8, 0 + }; + PrepareSRegToVRegMap(sreg_to_vreg_map); + + PrepareMIRs(mirs); + static const int32_t wide_sregs[] = { 5, 9 }; + MarkAsWideSRegs(wide_sregs); + PerformGVN_DCE(); + + ASSERT_EQ(arraysize(mirs), value_names_.size()); + static const size_t diff_indexes[] = { 0, 1, 2, 5, 6, 7, 9 }; + ExpectValueNamesNE(diff_indexes); + EXPECT_EQ(value_names_[0], value_names_[3]); + EXPECT_EQ(value_names_[2], value_names_[4]); + EXPECT_EQ(value_names_[5], value_names_[8]); + EXPECT_EQ(value_names_[6], value_names_[10]); + + static const bool eliminated[] = { + false, false, false, true, true, false, false, false, true, false, true, + }; + static_assert(arraysize(eliminated) == arraysize(mirs), "array size mismatch"); + for (size_t i = 0; i != arraysize(eliminated); ++i) { + bool actually_eliminated = (static_cast<int>(mirs_[i].dalvikInsn.opcode) == kMirOpNop); + EXPECT_EQ(eliminated[i], actually_eliminated) << i; + } +} + } // namespace art diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h index b25e967609..e0c56fcc82 100644 --- a/compiler/driver/compiler_driver-inl.h +++ b/compiler/driver/compiler_driver-inl.h @@ -233,11 +233,32 @@ inline bool CompilerDriver::IsStaticFieldInReferrerClass(mirror::Class* referrer return referrer_class == fields_class; } +inline bool CompilerDriver::CanAssumeClassIsInitialized(mirror::Class* klass) { + // Being loaded is a pre-requisite for being initialized but let's do the cheap check first. + // + // NOTE: When AOT compiling an app, we eagerly initialize app classes (and potentially their + // super classes in the boot image) but only those that have a trivial initialization, i.e. + // without <clinit>() or static values in the dex file for that class or any of its super + // classes. So while we could see the klass as initialized during AOT compilation and have + // it only loaded at runtime, the needed initialization would have to be trivial and + // unobservable from Java, so we may as well treat it as initialized. + if (!klass->IsInitialized()) { + return false; + } + return CanAssumeClassIsLoaded(klass); +} + +inline bool CompilerDriver::CanReferrerAssumeClassIsInitialized(mirror::Class* referrer_class, + mirror::Class* klass) { + return (referrer_class != nullptr && referrer_class->IsSubClass(klass)) || + CanAssumeClassIsInitialized(klass); +} + inline bool CompilerDriver::IsStaticFieldsClassInitialized(mirror::Class* referrer_class, ArtField* resolved_field) { DCHECK(resolved_field->IsStatic()); mirror::Class* fields_class = resolved_field->GetDeclaringClass(); - return fields_class == referrer_class || fields_class->IsInitialized(); + return CanReferrerAssumeClassIsInitialized(referrer_class, fields_class); } inline ArtMethod* CompilerDriver::ResolveMethod( @@ -394,7 +415,7 @@ inline bool CompilerDriver::IsMethodsClassInitialized(mirror::Class* referrer_cl return true; } mirror::Class* methods_class = resolved_method->GetDeclaringClass(); - return methods_class == referrer_class || methods_class->IsInitialized(); + return CanReferrerAssumeClassIsInitialized(referrer_class, methods_class); } } // namespace art diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 22fcf87524..84b6a52bda 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -659,7 +659,8 @@ void CompilerDriver::PreCompile(jobject class_loader, const std::vector<const De bool CompilerDriver::IsImageClass(const char* descriptor) const { if (!IsImage()) { - return true; + // NOTE: Currently unreachable, all callers check IsImage(). + return false; } else { return image_classes_->find(descriptor) != image_classes_->end(); } @@ -992,6 +993,24 @@ void CompilerDriver::UpdateImageClasses(TimingLogger* timings) { } } +bool CompilerDriver::CanAssumeClassIsLoaded(mirror::Class* klass) { + Runtime* runtime = Runtime::Current(); + if (!runtime->IsAotCompiler()) { + DCHECK(runtime->UseJit()); + // Having the klass reference here implies that the klass is already loaded. + return true; + } + if (!IsImage()) { + // Assume loaded only if klass is in the boot image. App classes cannot be assumed + // loaded because we don't even know what class loader will be used to load them. + bool class_in_image = runtime->GetHeap()->FindSpaceFromObject(klass, false)->IsImageSpace(); + return class_in_image; + } + std::string temp; + const char* descriptor = klass->GetDescriptor(&temp); + return IsImageClass(descriptor); +} + bool CompilerDriver::CanAssumeTypeIsPresentInDexCache(const DexFile& dex_file, uint32_t type_idx) { if (IsImage() && IsImageClass(dex_file.StringDataByIdx(dex_file.GetTypeId(type_idx).descriptor_idx_))) { diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index 68c905eb22..f737007308 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -501,6 +501,16 @@ class CompilerDriver { uint32_t field_idx) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // Can we assume that the klass is initialized? + bool CanAssumeClassIsInitialized(mirror::Class* klass) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + bool CanReferrerAssumeClassIsInitialized(mirror::Class* referrer_class, mirror::Class* klass) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + + // Can we assume that the klass is loaded? + bool CanAssumeClassIsLoaded(mirror::Class* klass) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // These flags are internal to CompilerDriver for collecting INVOKE resolution statistics. // The only external contract is that unresolved method has flags 0 and resolved non-0. enum { diff --git a/compiler/optimizing/dead_code_elimination.cc b/compiler/optimizing/dead_code_elimination.cc index 17a006cc3a..fdfe518e95 100644 --- a/compiler/optimizing/dead_code_elimination.cc +++ b/compiler/optimizing/dead_code_elimination.cc @@ -122,10 +122,6 @@ void HDeadCodeElimination::RemoveDeadInstructions() { if (!inst->HasSideEffects() && !inst->CanThrow() && !inst->IsSuspendCheck() - // The current method needs to stay in the graph in case of inlining. - // It is always passed anyway, and keeping it in the graph does not - // affect the generated code. - && !inst->IsCurrentMethod() // If we added an explicit barrier then we should keep it. && !inst->IsMemoryBarrier() && !inst->HasUses()) { diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 830d84ba50..68c197e607 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -299,7 +299,10 @@ HNullConstant* HGraph::GetNullConstant() { } HCurrentMethod* HGraph::GetCurrentMethod() { - if (cached_current_method_ == nullptr) { + // For simplicity, don't bother reviving the cached current method if it is + // not null and not in a block. Otherwise, we need to clear the instruction + // id and/or any invariants the graph is assuming when adding new instructions. + if ((cached_current_method_ == nullptr) || (cached_current_method_->GetBlock() == nullptr)) { cached_current_method_ = new (arena_) HCurrentMethod( Is64BitInstructionSet(instruction_set_) ? Primitive::kPrimLong : Primitive::kPrimInt); if (entry_block_->GetFirstInstruction() == nullptr) { diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 7ef69559de..9443653db7 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -335,6 +335,7 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { } // If not found or previously deleted, create and cache a new instruction. + // Don't bother reviving a previously deleted instruction, for simplicity. if (constant == nullptr || constant->GetBlock() == nullptr) { constant = new (arena_) InstructionType(value); cache->Overwrite(value, constant); diff --git a/runtime/oat.h b/runtime/oat.h index 604e16171d..000ae8ed5d 100644 --- a/runtime/oat.h +++ b/runtime/oat.h @@ -32,7 +32,7 @@ class InstructionSetFeatures; class PACKED(4) OatHeader { public: static constexpr uint8_t kOatMagic[] = { 'o', 'a', 't', '\n' }; - static constexpr uint8_t kOatVersion[] = { '0', '6', '3', '\0' }; + static constexpr uint8_t kOatVersion[] = { '0', '6', '4', '\0' }; static constexpr const char* kImageLocationKey = "image-location"; static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline"; diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk index 469df1f2b6..c2380cc668 100644 --- a/test/Android.run-test.mk +++ b/test/Android.run-test.mk @@ -377,8 +377,7 @@ TEST_ART_BROKEN_JIT_RUN_TESTS := # Known broken tests for the default compiler (Quick). TEST_ART_BROKEN_DEFAULT_RUN_TESTS := \ - 457-regs \ - 496-checker-inlining-and-class-loader + 457-regs ifneq (,$(filter default,$(COMPILER_TYPES))) ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \ |