diff options
25 files changed, 383 insertions, 142 deletions
diff --git a/compiler/dex/inline_method_analyser.cc b/compiler/dex/inline_method_analyser.cc index 925863ef0e..518b0ece73 100644 --- a/compiler/dex/inline_method_analyser.cc +++ b/compiler/dex/inline_method_analyser.cc @@ -302,7 +302,8 @@ bool DoAnalyseConstructor(const DexFile::CodeItem* code_item, uint16_t this_vreg = code_item->registers_size_ - code_item->ins_size_; uint16_t zero_vreg_mask = 0u; - for (const Instruction& instruction : code_item->Instructions()) { + for (const DexInstructionPcPair& pair : code_item->Instructions()) { + const Instruction& instruction = pair.Inst(); if (instruction.Opcode() == Instruction::RETURN_VOID) { break; } else if (instruction.Opcode() == Instruction::INVOKE_DIRECT) { diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc index 9c5b63232e..df75e07c3f 100644 --- a/compiler/dex/verified_method.cc +++ b/compiler/dex/verified_method.cc @@ -64,12 +64,11 @@ void VerifiedMethod::GenerateSafeCastSet(verifier::MethodVerifier* method_verifi if (method_verifier->HasFailures()) { return; } - IterationRange<DexInstructionIterator> instructions = method_verifier->CodeItem()->Instructions(); - for (auto it = instructions.begin(); it != instructions.end(); ++it) { - const Instruction& inst = *it; + for (const DexInstructionPcPair& pair : method_verifier->CodeItem()->Instructions()) { + const Instruction& inst = pair.Inst(); const Instruction::Code code = inst.Opcode(); if (code == Instruction::CHECK_CAST) { - const uint32_t dex_pc = it.GetDexPC(instructions.begin()); + const uint32_t dex_pc = pair.DexPc(); if (!method_verifier->GetInstructionFlags(dex_pc).IsVisited()) { // Do not attempt to quicken this instruction, it's unreachable anyway. continue; diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 3d4da5edcf..bd78af4f4d 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -737,13 +737,13 @@ static void ResolveConstStrings(Handle<mirror::DexCache> dex_cache, } ClassLinker* const class_linker = Runtime::Current()->GetClassLinker(); - for (const Instruction& inst : code_item->Instructions()) { - switch (inst.Opcode()) { + for (const DexInstructionPcPair& inst : code_item->Instructions()) { + switch (inst->Opcode()) { case Instruction::CONST_STRING: case Instruction::CONST_STRING_JUMBO: { - dex::StringIndex string_index((inst.Opcode() == Instruction::CONST_STRING) - ? inst.VRegB_21c() - : inst.VRegB_31c()); + dex::StringIndex string_index((inst->Opcode() == Instruction::CONST_STRING) + ? inst->VRegB_21c() + : inst->VRegB_31c()); mirror::String* string = class_linker->ResolveString(dex_file, string_index, dex_cache); CHECK(string != nullptr) << "Could not allocate a string when forcing determinism"; break; @@ -2417,14 +2417,14 @@ class InitializeClassVisitor : public CompilationVisitor { if (clinit != nullptr) { const DexFile::CodeItem* code_item = clinit->GetCodeItem(); DCHECK(code_item != nullptr); - for (const Instruction& inst : code_item->Instructions()) { - if (inst.Opcode() == Instruction::CONST_STRING) { + for (const DexInstructionPcPair& inst : code_item->Instructions()) { + if (inst->Opcode() == Instruction::CONST_STRING) { ObjPtr<mirror::String> s = class_linker->ResolveString( - *dex_file, dex::StringIndex(inst.VRegB_21c()), h_dex_cache); + *dex_file, dex::StringIndex(inst->VRegB_21c()), h_dex_cache); CHECK(s != nullptr); - } else if (inst.Opcode() == Instruction::CONST_STRING_JUMBO) { + } else if (inst->Opcode() == Instruction::CONST_STRING_JUMBO) { ObjPtr<mirror::String> s = class_linker->ResolveString( - *dex_file, dex::StringIndex(inst.VRegB_31c()), h_dex_cache); + *dex_file, dex::StringIndex(inst->VRegB_31c()), h_dex_cache); CHECK(s != nullptr); } } diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index 5625f04726..015a6a04d3 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -919,10 +919,9 @@ static void CheckLoopEntriesCanBeUsedForOsr(const HGraph& graph, } ArenaVector<size_t> covered( loop_headers.size(), 0, graph.GetAllocator()->Adapter(kArenaAllocMisc)); - IterationRange<DexInstructionIterator> instructions = code_item.Instructions(); - for (auto it = instructions.begin(); it != instructions.end(); ++it) { - const uint32_t dex_pc = it.GetDexPC(instructions.begin()); - const Instruction& instruction = *it; + for (const DexInstructionPcPair& pair : code_item.Instructions()) { + const uint32_t dex_pc = pair.DexPc(); + const Instruction& instruction = pair.Inst(); if (instruction.IsBranch()) { uint32_t target = dex_pc + instruction.GetTargetOffset(); CheckCovers(target, graph, code_info, loop_headers, &covered); diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index b3fed079d8..60c7a8f2ef 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -9245,6 +9245,16 @@ void InstructionCodeGeneratorMIPS::VisitClassTableGet(HClassTableGet* instructio } } +void LocationsBuilderMIPS::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + +void InstructionCodeGeneratorMIPS::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + #undef __ #undef QUICK_ENTRY_POINT diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 53a7f26c81..5292638c07 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -7147,5 +7147,15 @@ void InstructionCodeGeneratorMIPS64::VisitClassTableGet(HClassTableGet* instruct } } +void LocationsBuilderMIPS64::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + +void InstructionCodeGeneratorMIPS64::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + } // namespace mips64 } // namespace art diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index f84dd0045e..a1500825f8 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -7825,6 +7825,16 @@ void CodeGeneratorX86::EmitJitRootPatches(uint8_t* code, const uint8_t* roots_da } } +void LocationsBuilderX86::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + +void InstructionCodeGeneratorX86::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + #undef __ } // namespace x86 diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index 16d1f183a1..db7e53e74b 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -6833,6 +6833,16 @@ void InstructionCodeGeneratorX86_64::VisitPackedSwitch(HPackedSwitch* switch_ins __ jmp(temp_reg); } +void LocationsBuilderX86_64::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + +void InstructionCodeGeneratorX86_64::VisitIntermediateAddress(HIntermediateAddress* instruction + ATTRIBUTE_UNUSED) { + LOG(FATAL) << "Unreachable"; +} + void CodeGeneratorX86_64::Load32BitValue(CpuRegister dest, int32_t value) { if (value == 0) { __ xorl(dest, dest); diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc index 902985e4ee..0f0be20961 100644 --- a/compiler/optimizing/instruction_builder.cc +++ b/compiler/optimizing/instruction_builder.cc @@ -382,16 +382,18 @@ ArenaBitVector* HInstructionBuilder::FindNativeDebugInfoLocations() { dex_file_->DecodeDebugPositionInfo(&code_item_, Callback::Position, locations); // Instruction-specific tweaks. IterationRange<DexInstructionIterator> instructions = code_item_.Instructions(); - for (const Instruction& inst : instructions) { - switch (inst.Opcode()) { + for (DexInstructionIterator it = instructions.begin(); it != instructions.end(); ++it) { + switch (it->Opcode()) { case Instruction::MOVE_EXCEPTION: { // Stop in native debugger after the exception has been moved. // The compiler also expects the move at the start of basic block so // we do not want to interfere by inserting native-debug-info before it. - locations->ClearBit(inst.GetDexPc(code_item_.insns_)); - const Instruction* next = inst.Next(); - if (DexInstructionIterator(next) != instructions.end()) { - locations->SetBit(next->GetDexPc(code_item_.insns_)); + locations->ClearBit(it.DexPc()); + DexInstructionIterator next = it; + ++next; + DCHECK(next != it); + if (next != instructions.end()) { + locations->SetBit(next.DexPc()); } break; } diff --git a/compiler/optimizing/load_store_analysis.h b/compiler/optimizing/load_store_analysis.h index 5940ee755f..5a1df45914 100644 --- a/compiler/optimizing/load_store_analysis.h +++ b/compiler/optimizing/load_store_analysis.h @@ -196,8 +196,12 @@ class HeapLocationCollector : public HGraphVisitor { } HInstruction* HuntForOriginalReference(HInstruction* ref) const { + // An original reference can be transformed by instructions like: + // i0 NewArray + // i1 HInstruction(i0) <-- NullCheck, BoundType, IntermediateAddress. + // i2 ArrayGet(i1, index) DCHECK(ref != nullptr); - while (ref->IsNullCheck() || ref->IsBoundType()) { + while (ref->IsNullCheck() || ref->IsBoundType() || ref->IsIntermediateAddress()) { ref = ref->InputAt(0); } return ref; diff --git a/compiler/optimizing/load_store_analysis_test.cc b/compiler/optimizing/load_store_analysis_test.cc index 86696d02a1..b41e1e4d00 100644 --- a/compiler/optimizing/load_store_analysis_test.cc +++ b/compiler/optimizing/load_store_analysis_test.cc @@ -389,4 +389,68 @@ TEST_F(LoadStoreAnalysisTest, ArrayIndexCalculationOverflowTest) { ASSERT_FALSE(heap_location_collector.MayAlias(loc1, loc2)); } +TEST_F(LoadStoreAnalysisTest, TestHuntOriginalRef) { + HBasicBlock* entry = new (GetAllocator()) HBasicBlock(graph_); + graph_->AddBlock(entry); + graph_->SetEntryBlock(entry); + + // Different ways where orignal array reference are transformed & passed to ArrayGet. + // ParameterValue --> ArrayGet + // ParameterValue --> BoundType --> ArrayGet + // ParameterValue --> BoundType --> NullCheck --> ArrayGet + // ParameterValue --> BoundType --> NullCheck --> IntermediateAddress --> ArrayGet + HInstruction* c1 = graph_->GetIntConstant(1); + HInstruction* array = new (GetAllocator()) HParameterValue(graph_->GetDexFile(), + dex::TypeIndex(0), + 0, + DataType::Type::kReference); + HInstruction* array_get1 = new (GetAllocator()) HArrayGet(array, + c1, + DataType::Type::kInt32, + 0); + + HInstruction* bound_type = new (GetAllocator()) HBoundType(array); + HInstruction* array_get2 = new (GetAllocator()) HArrayGet(bound_type, + c1, + DataType::Type::kInt32, + 0); + + HInstruction* null_check = new (GetAllocator()) HNullCheck(bound_type, 0); + HInstruction* array_get3 = new (GetAllocator()) HArrayGet(null_check, + c1, + DataType::Type::kInt32, + 0); + + HInstruction* inter_addr = new (GetAllocator()) HIntermediateAddress(null_check, c1, 0); + HInstruction* array_get4 = new (GetAllocator()) HArrayGet(inter_addr, + c1, + DataType::Type::kInt32, + 0); + entry->AddInstruction(array); + entry->AddInstruction(array_get1); + entry->AddInstruction(bound_type); + entry->AddInstruction(array_get2); + entry->AddInstruction(null_check); + entry->AddInstruction(array_get3); + entry->AddInstruction(inter_addr); + entry->AddInstruction(array_get4); + + HeapLocationCollector heap_location_collector(graph_); + heap_location_collector.VisitBasicBlock(entry); + + // Test that the HeapLocationCollector should be able to tell + // that there is only ONE array location, no matter how many + // times the original reference has been transformed by BoundType, + // NullCheck, IntermediateAddress, etc. + ASSERT_EQ(heap_location_collector.GetNumberOfHeapLocations(), 1U); + size_t loc1 = heap_location_collector.GetArrayAccessHeapLocation(array, c1); + size_t loc2 = heap_location_collector.GetArrayAccessHeapLocation(bound_type, c1); + size_t loc3 = heap_location_collector.GetArrayAccessHeapLocation(null_check, c1); + size_t loc4 = heap_location_collector.GetArrayAccessHeapLocation(inter_addr, c1); + ASSERT_TRUE(loc1 != HeapLocationCollector::kHeapLocationNotFound); + ASSERT_EQ(loc1, loc2); + ASSERT_EQ(loc1, loc3); + ASSERT_EQ(loc1, loc4); +} + } // namespace art diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 88609ea790..29c78a1e34 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -1334,6 +1334,7 @@ class HLoopInformationOutwardIterator : public ValueObject { M(InstanceFieldSet, Instruction) \ M(InstanceOf, Instruction) \ M(IntConstant, Constant) \ + M(IntermediateAddress, Instruction) \ M(InvokeUnresolved, Invoke) \ M(InvokeInterface, Invoke) \ M(InvokeStaticOrDirect, Invoke) \ @@ -1418,7 +1419,6 @@ class HLoopInformationOutwardIterator : public ValueObject { M(BitwiseNegatedRight, Instruction) \ M(DataProcWithShifterOp, Instruction) \ M(MultiplyAccumulate, Instruction) \ - M(IntermediateAddress, Instruction) \ M(IntermediateAddressIndex, Instruction) #endif @@ -6966,6 +6966,38 @@ class HParallelMove FINAL : public HTemplateInstruction<0> { DISALLOW_COPY_AND_ASSIGN(HParallelMove); }; +// This instruction computes an intermediate address pointing in the 'middle' of an object. The +// result pointer cannot be handled by GC, so extra care is taken to make sure that this value is +// never used across anything that can trigger GC. +// The result of this instruction is not a pointer in the sense of `DataType::Type::kreference`. +// So we represent it by the type `DataType::Type::kInt`. +class HIntermediateAddress FINAL : public HExpression<2> { + public: + HIntermediateAddress(HInstruction* base_address, HInstruction* offset, uint32_t dex_pc) + : HExpression(DataType::Type::kInt32, SideEffects::DependsOnGC(), dex_pc) { + DCHECK_EQ(DataType::Size(DataType::Type::kInt32), + DataType::Size(DataType::Type::kReference)) + << "kPrimInt and kPrimNot have different sizes."; + SetRawInputAt(0, base_address); + SetRawInputAt(1, offset); + } + + bool CanBeMoved() const OVERRIDE { return true; } + bool InstructionDataEquals(const HInstruction* other ATTRIBUTE_UNUSED) const OVERRIDE { + return true; + } + bool IsActualObject() const OVERRIDE { return false; } + + HInstruction* GetBaseAddress() const { return InputAt(0); } + HInstruction* GetOffset() const { return InputAt(1); } + + DECLARE_INSTRUCTION(IntermediateAddress); + + private: + DISALLOW_COPY_AND_ASSIGN(HIntermediateAddress); +}; + + } // namespace art #include "nodes_vector.h" diff --git a/compiler/optimizing/nodes_shared.h b/compiler/optimizing/nodes_shared.h index 14cbf85c3f..7b4f5f7cbb 100644 --- a/compiler/optimizing/nodes_shared.h +++ b/compiler/optimizing/nodes_shared.h @@ -118,38 +118,6 @@ class HBitwiseNegatedRight FINAL : public HBinaryOperation { DISALLOW_COPY_AND_ASSIGN(HBitwiseNegatedRight); }; - -// This instruction computes an intermediate address pointing in the 'middle' of an object. The -// result pointer cannot be handled by GC, so extra care is taken to make sure that this value is -// never used across anything that can trigger GC. -// The result of this instruction is not a pointer in the sense of `DataType::Type::kreference`. -// So we represent it by the type `DataType::Type::kInt`. -class HIntermediateAddress FINAL : public HExpression<2> { - public: - HIntermediateAddress(HInstruction* base_address, HInstruction* offset, uint32_t dex_pc) - : HExpression(DataType::Type::kInt32, SideEffects::DependsOnGC(), dex_pc) { - DCHECK_EQ(DataType::Size(DataType::Type::kInt32), - DataType::Size(DataType::Type::kReference)) - << "kPrimInt and kPrimNot have different sizes."; - SetRawInputAt(0, base_address); - SetRawInputAt(1, offset); - } - - bool CanBeMoved() const OVERRIDE { return true; } - bool InstructionDataEquals(const HInstruction* other ATTRIBUTE_UNUSED) const OVERRIDE { - return true; - } - bool IsActualObject() const OVERRIDE { return false; } - - HInstruction* GetBaseAddress() const { return InputAt(0); } - HInstruction* GetOffset() const { return InputAt(1); } - - DECLARE_INSTRUCTION(IntermediateAddress); - - private: - DISALLOW_COPY_AND_ASSIGN(HIntermediateAddress); -}; - // This instruction computes part of the array access offset (data and index offset). // // For array accesses the element address has the following structure: diff --git a/dexlayout/dex_ir.cc b/dexlayout/dex_ir.cc index 23c3a5ca93..3edb0a44f2 100644 --- a/dexlayout/dex_ir.cc +++ b/dexlayout/dex_ir.cc @@ -173,11 +173,11 @@ static bool GetIdsFromByteCode(Collections& collections, // In case the instruction goes past the end of the code item, make sure to not process it. SafeDexInstructionIterator next = it; ++next; - if (next.IsErrorState() || next > instructions.end()) { + if (next.IsErrorState()) { break; } has_id |= GetIdFromInstruction(collections, - it.Inst(), + &it.Inst(), type_ids, string_ids, method_ids, diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h index 99a66f348c..179d3b96e0 100644 --- a/dexlayout/dex_ir.h +++ b/dexlayout/dex_ir.h @@ -952,8 +952,8 @@ class CodeItem : public Item { void Accept(AbstractDispatcher* dispatch) { dispatch->Dispatch(this); } IterationRange<DexInstructionIterator> Instructions() const { - return MakeIterationRange(DexInstructionIterator(Insns()), - DexInstructionIterator(Insns() + InsnsSize())); + return MakeIterationRange(DexInstructionIterator(Insns(), 0u), + DexInstructionIterator(Insns(), InsnsSize())); } private: diff --git a/dexlayout/dexlayout.cc b/dexlayout/dexlayout.cc index 9a2ab665ba..dd2e809a92 100644 --- a/dexlayout/dexlayout.cc +++ b/dexlayout/dexlayout.cc @@ -1054,15 +1054,13 @@ void DexLayout::DumpBytecodes(uint32_t idx, const dex_ir::CodeItem* code, uint32 code_offset, code_offset, dot.c_str(), name, type_descriptor.c_str()); // Iterate over all instructions. - IterationRange<DexInstructionIterator> instructions = code->Instructions(); - for (auto inst = instructions.begin(); inst != instructions.end(); ++inst) { - const uint32_t dex_pc = inst.GetDexPC(instructions.begin()); + for (const DexInstructionPcPair& inst : code->Instructions()) { const uint32_t insn_width = inst->SizeInCodeUnits(); if (insn_width == 0) { - fprintf(stderr, "GLITCH: zero-width instruction at idx=0x%04x\n", dex_pc); + fprintf(stderr, "GLITCH: zero-width instruction at idx=0x%04x\n", inst.DexPc()); break; } - DumpInstruction(code, code_offset, dex_pc, insn_width, &*inst); + DumpInstruction(code, code_offset, inst.DexPc(), insn_width, &inst.Inst()); } // for } diff --git a/dexlayout/dexlayout_test.cc b/dexlayout/dexlayout_test.cc index 38d3c6ec07..08673056d9 100644 --- a/dexlayout/dexlayout_test.cc +++ b/dexlayout/dexlayout_test.cc @@ -724,7 +724,7 @@ TEST_F(DexLayoutTest, CodeItemOverrun) { } if (last_instruction->SizeInCodeUnits() == 1) { // Set the opcode to something that will go past the end of the code item. - const_cast<Instruction*>(last_instruction.Inst())->SetOpcode( + const_cast<Instruction&>(last_instruction.Inst()).SetOpcode( Instruction::CONST_STRING_JUMBO); mutated_successfully = true; // Test that the safe iterator doesn't go past the end. diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index 21eb207559..3b0476ba46 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -1000,16 +1000,16 @@ class OatDumper { dex_code_bytes_ += code_item->insns_size_in_code_units_ * sizeof(code_ptr[0]); } - for (const Instruction& inst : code_item->Instructions()) { - switch (inst.Opcode()) { + for (const DexInstructionPcPair& inst : code_item->Instructions()) { + switch (inst->Opcode()) { case Instruction::CONST_STRING: { - const dex::StringIndex string_index(inst.VRegB_21c()); + const dex::StringIndex string_index(inst->VRegB_21c()); unique_string_ids_from_code_.insert(StringReference(&dex_file, string_index)); ++num_string_ids_from_code_; break; } case Instruction::CONST_STRING_JUMBO: { - const dex::StringIndex string_index(inst.VRegB_31c()); + const dex::StringIndex string_index(inst->VRegB_31c()); unique_string_ids_from_code_.insert(StringReference(&dex_file, string_index)); ++num_string_ids_from_code_; break; @@ -1624,11 +1624,9 @@ class OatDumper { void DumpDexCode(std::ostream& os, const DexFile& dex_file, const DexFile::CodeItem* code_item) { if (code_item != nullptr) { - IterationRange<DexInstructionIterator> instructions = code_item->Instructions(); - for (auto it = instructions.begin(); it != instructions.end(); ++it) { - const size_t dex_pc = it.GetDexPC(instructions.begin()); - os << StringPrintf("0x%04zx: ", dex_pc) << it->DumpHexLE(5) - << StringPrintf("\t| %s\n", it->DumpString(&dex_file).c_str()); + for (const DexInstructionPcPair& inst : code_item->Instructions()) { + os << StringPrintf("0x%04x: ", inst.DexPc()) << inst->DumpHexLE(5) + << StringPrintf("\t| %s\n", inst->DumpString(&dex_file).c_str()); } } } diff --git a/runtime/dex_file.h b/runtime/dex_file.h index c895e0d1da..c0e0fbcf8f 100644 --- a/runtime/dex_file.h +++ b/runtime/dex_file.h @@ -305,8 +305,8 @@ class DexFile { // Raw code_item. struct CodeItem { IterationRange<DexInstructionIterator> Instructions() const { - return { DexInstructionIterator(insns_), - DexInstructionIterator(insns_ + insns_size_in_code_units_)}; + return { DexInstructionIterator(insns_, 0u), + DexInstructionIterator(insns_, insns_size_in_code_units_) }; } const Instruction& InstructionAt(uint32_t dex_pc) const { diff --git a/runtime/dex_instruction_iterator.h b/runtime/dex_instruction_iterator.h index 9e4dea34bb..f77908cffc 100644 --- a/runtime/dex_instruction_iterator.h +++ b/runtime/dex_instruction_iterator.h @@ -24,33 +24,69 @@ namespace art { +class DexInstructionPcPair { + public: + ALWAYS_INLINE const Instruction& Inst() const { + return *Instruction::At(instructions_ + DexPc()); + } + + ALWAYS_INLINE const Instruction* operator->() const { + return &Inst(); + } + + ALWAYS_INLINE uint32_t DexPc() const { + return dex_pc_; + } + + ALWAYS_INLINE const uint16_t* Instructions() const { + return instructions_; + } + + protected: + explicit DexInstructionPcPair(const uint16_t* instructions, uint32_t dex_pc) + : instructions_(instructions), dex_pc_(dex_pc) {} + + const uint16_t* instructions_ = nullptr; + uint32_t dex_pc_ = 0; + + friend class DexInstructionIteratorBase; + friend class DexInstructionIterator; + friend class SafeDexInstructionIterator; +}; + // Base helper class to prevent duplicated comparators. -class DexInstructionIteratorBase : public std::iterator<std::forward_iterator_tag, Instruction> { +class DexInstructionIteratorBase : public + std::iterator<std::forward_iterator_tag, DexInstructionPcPair> { public: - using value_type = std::iterator<std::forward_iterator_tag, Instruction>::value_type; + using value_type = std::iterator<std::forward_iterator_tag, DexInstructionPcPair>::value_type; using difference_type = std::iterator<std::forward_iterator_tag, value_type>::difference_type; DexInstructionIteratorBase() = default; - explicit DexInstructionIteratorBase(const value_type* inst) : inst_(inst) {} + explicit DexInstructionIteratorBase(const Instruction* inst, uint32_t dex_pc) + : data_(reinterpret_cast<const uint16_t*>(inst), dex_pc) {} - const value_type* Inst() const { - return inst_; + const Instruction& Inst() const { + return data_.Inst(); } // Return the dex pc for an iterator compared to the code item begin. - uint32_t GetDexPC(const DexInstructionIteratorBase& code_item_begin) { - return reinterpret_cast<const uint16_t*>(inst_) - - reinterpret_cast<const uint16_t*>(code_item_begin.inst_); + ALWAYS_INLINE uint32_t DexPc() const { + return data_.DexPc(); + } + + // Instructions from the start of the code item. + ALWAYS_INLINE const uint16_t* Instructions() const { + return data_.Instructions(); } protected: - const value_type* inst_ = nullptr; + DexInstructionPcPair data_; }; - static ALWAYS_INLINE inline bool operator==(const DexInstructionIteratorBase& lhs, const DexInstructionIteratorBase& rhs) { - return lhs.Inst() == rhs.Inst(); + DCHECK_EQ(lhs.Instructions(), rhs.Instructions()) << "Comparing different code items."; + return lhs.DexPc() == rhs.DexPc(); } static inline bool operator!=(const DexInstructionIteratorBase& lhs, @@ -60,7 +96,8 @@ static inline bool operator!=(const DexInstructionIteratorBase& lhs, static inline bool operator<(const DexInstructionIteratorBase& lhs, const DexInstructionIteratorBase& rhs) { - return lhs.Inst() < rhs.Inst(); + DCHECK_EQ(lhs.Instructions(), rhs.Instructions()) << "Comparing different code items."; + return lhs.DexPc() < rhs.DexPc(); } static inline bool operator>(const DexInstructionIteratorBase& lhs, @@ -78,18 +115,17 @@ static inline bool operator>=(const DexInstructionIteratorBase& lhs, return !(lhs < rhs); } +// A helper class for a code_item's instructions using range based for loop syntax. class DexInstructionIterator : public DexInstructionIteratorBase { public: - using value_type = std::iterator<std::forward_iterator_tag, Instruction>::value_type; - using difference_type = std::iterator<std::forward_iterator_tag, value_type>::difference_type; using DexInstructionIteratorBase::DexInstructionIteratorBase; - explicit DexInstructionIterator(const uint16_t* inst) - : DexInstructionIteratorBase(value_type::At(inst)) {} + explicit DexInstructionIterator(const uint16_t* inst, uint32_t dex_pc) + : DexInstructionIteratorBase(Instruction::At(inst), dex_pc) {} // Value after modification. DexInstructionIterator& operator++() { - inst_ = inst_->Next(); + data_.dex_pc_ += Inst().SizeInCodeUnits(); return *this; } @@ -101,38 +137,47 @@ class DexInstructionIterator : public DexInstructionIteratorBase { } const value_type& operator*() const { - return *inst_; + return data_; } - const value_type* operator->() const { - return &**this; + const Instruction* operator->() const { + return &data_.Inst(); + } + + // Return the dex pc for the iterator. + ALWAYS_INLINE uint32_t DexPc() const { + return data_.DexPc(); } }; +// A safe version of DexInstructionIterator that is guaranteed to not go past the end of the code +// item. class SafeDexInstructionIterator : public DexInstructionIteratorBase { public: explicit SafeDexInstructionIterator(const DexInstructionIteratorBase& start, const DexInstructionIteratorBase& end) - : DexInstructionIteratorBase(start.Inst()) - , end_(end.Inst()) {} + : DexInstructionIteratorBase(&start.Inst(), start.DexPc()) + , num_code_units_(end.DexPc()) { + DCHECK_EQ(start.Instructions(), end.Instructions()) + << "start and end must be in the same code item."; + } // Value after modification, does not read past the end of the allowed region. May increment past // the end of the code item though. SafeDexInstructionIterator& operator++() { AssertValid(); - const size_t size_code_units = Inst()->CodeUnitsRequiredForSizeComputation(); - const size_t available = reinterpret_cast<const uint16_t*>(end_) - - reinterpret_cast<const uint16_t*>(Inst()); + const size_t size_code_units = Inst().CodeUnitsRequiredForSizeComputation(); + const size_t available = NumCodeUnits() - DexPc(); if (UNLIKELY(size_code_units > available)) { error_state_ = true; return *this; } - const size_t instruction_size = inst_->SizeInCodeUnits(); - if (UNLIKELY(instruction_size > available)) { + const size_t instruction_code_units = Inst().SizeInCodeUnits(); + if (UNLIKELY(instruction_code_units > available)) { error_state_ = true; return *this; } - inst_ = inst_->RelativeAt(instruction_size); + data_.dex_pc_ += instruction_code_units; return *this; } @@ -145,12 +190,21 @@ class SafeDexInstructionIterator : public DexInstructionIteratorBase { const value_type& operator*() const { AssertValid(); - return *inst_; + return data_; } - const value_type* operator->() const { + const Instruction* operator->() const { AssertValid(); - return &**this; + return &data_.Inst(); + } + + // Return the current instruction of the iterator. + ALWAYS_INLINE const Instruction& Inst() const { + return data_.Inst(); + } + + const uint16_t* Instructions() const { + return data_.Instructions(); } // Returns true if the iterator is in an error state. This occurs when an instruction couldn't @@ -162,10 +216,14 @@ class SafeDexInstructionIterator : public DexInstructionIteratorBase { private: ALWAYS_INLINE void AssertValid() const { DCHECK(!IsErrorState()); - DCHECK_LT(Inst(), end_); + DCHECK_LT(DexPc(), NumCodeUnits()); + } + + ALWAYS_INLINE uint32_t NumCodeUnits() const { + return num_code_units_; } - const value_type* end_ = nullptr; + const uint32_t num_code_units_ = 0; bool error_state_ = false; }; diff --git a/runtime/dex_instruction_test.cc b/runtime/dex_instruction_test.cc index 48ed027882..c944085b9e 100644 --- a/runtime/dex_instruction_test.cc +++ b/runtime/dex_instruction_test.cc @@ -74,7 +74,7 @@ TEST(Instruction, PropertiesOf45cc) { Build45cc(4u /* num_vregs */, 16u /* method_idx */, 32u /* proto_idx */, 0xcafe /* arg_regs */, instruction); - DexInstructionIterator ins(instruction); + DexInstructionIterator ins(instruction, /*dex_pc*/ 0u); ASSERT_EQ(4u, ins->SizeInCodeUnits()); ASSERT_TRUE(ins->HasVRegA()); @@ -109,7 +109,7 @@ TEST(Instruction, PropertiesOf4rcc) { Build4rcc(4u /* num_vregs */, 16u /* method_idx */, 32u /* proto_idx */, 0xcafe /* arg_regs */, instruction); - DexInstructionIterator ins(instruction); + DexInstructionIterator ins(instruction, /*dex_pc*/ 0u); ASSERT_EQ(4u, ins->SizeInCodeUnits()); ASSERT_TRUE(ins->HasVRegA()); @@ -155,7 +155,7 @@ static std::string DumpInst35c(Instruction::Code code, std::vector<uint16_t> args) { uint16_t inst[6] = {}; Build35c(inst, code, method_idx, args); - return DexInstructionIterator(inst)->DumpString(nullptr); + return Instruction::At(inst)->DumpString(nullptr); } TEST(Instruction, DumpString) { diff --git a/runtime/interpreter/unstarted_runtime_test.cc b/runtime/interpreter/unstarted_runtime_test.cc index fb378251ad..9db5f88dab 100644 --- a/runtime/interpreter/unstarted_runtime_test.cc +++ b/runtime/interpreter/unstarted_runtime_test.cc @@ -393,7 +393,6 @@ TEST_F(UnstartedRuntimeTest, StringInit) { // create instruction data for invoke-direct {v0, v1} of method with fake index uint16_t inst_data[3] = { 0x2070, 0x0000, 0x0010 }; - DexInstructionIterator inst(inst_data); JValue result; ShadowFrame* shadow_frame = ShadowFrame::CreateDeoptimizedFrame(10, nullptr, method, 0); @@ -403,7 +402,12 @@ TEST_F(UnstartedRuntimeTest, StringInit) { shadow_frame->SetVRegReference(0, reference_empty_string); shadow_frame->SetVRegReference(1, string_arg); - interpreter::DoCall<false, false>(method, self, *shadow_frame, &*inst, inst_data[0], &result); + interpreter::DoCall<false, false>(method, + self, + *shadow_frame, + Instruction::At(inst_data), + inst_data[0], + &result); mirror::String* string_result = reinterpret_cast<mirror::String*>(result.GetL()); EXPECT_EQ(string_arg->GetLength(), string_result->GetLength()); @@ -1027,12 +1031,16 @@ TEST_F(UnstartedRuntimeTest, FloatConversion) { // create instruction data for invoke-direct {v0, v1} of method with fake index uint16_t inst_data[3] = { 0x2070, 0x0000, 0x0010 }; - DexInstructionIterator inst(inst_data); JValue result; ShadowFrame* shadow_frame = ShadowFrame::CreateDeoptimizedFrame(10, nullptr, method, 0); shadow_frame->SetVRegDouble(0, 1.23); - interpreter::DoCall<false, false>(method, self, *shadow_frame, &*inst, inst_data[0], &result); + interpreter::DoCall<false, false>(method, + self, + *shadow_frame, + Instruction::At(inst_data), + inst_data[0], + &result); ObjPtr<mirror::String> string_result = reinterpret_cast<mirror::String*>(result.GetL()); ASSERT_TRUE(string_result != nullptr); @@ -1187,12 +1195,11 @@ class UnstartedClassForNameTest : public UnstartedRuntimeTest { // create instruction data for invoke-direct {v0} of method with fake index uint16_t inst_data[3] = { 0x1070, 0x0000, 0x0010 }; - DexInstructionIterator inst(inst_data); interpreter::DoCall<false, false>(boot_cp_init, self, *shadow_frame, - &*inst, + Instruction::At(inst_data), inst_data[0], &result); CHECK(!self->IsExceptionPending()); diff --git a/runtime/jit/profiling_info.cc b/runtime/jit/profiling_info.cc index ad013244c3..1344ca05b4 100644 --- a/runtime/jit/profiling_info.cc +++ b/runtime/jit/profiling_info.cc @@ -45,9 +45,7 @@ bool ProfilingInfo::Create(Thread* self, ArtMethod* method, bool retry_allocatio std::vector<uint32_t> entries; - IterationRange<DexInstructionIterator> instructions = method->GetCodeItem()->Instructions(); - for (auto inst = instructions.begin(); inst != instructions.end(); ++inst) { - const uint32_t dex_pc = inst.GetDexPC(instructions.begin()); + for (const DexInstructionPcPair& inst : method->GetCodeItem()->Instructions()) { switch (inst->Opcode()) { case Instruction::INVOKE_VIRTUAL: case Instruction::INVOKE_VIRTUAL_RANGE: @@ -55,7 +53,7 @@ bool ProfilingInfo::Create(Thread* self, ArtMethod* method, bool retry_allocatio case Instruction::INVOKE_VIRTUAL_RANGE_QUICK: case Instruction::INVOKE_INTERFACE: case Instruction::INVOKE_INTERFACE_RANGE: - entries.push_back(dex_pc); + entries.push_back(inst.DexPc()); break; default: diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 38c893bd5d..a84d0bada7 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -617,8 +617,8 @@ void MethodVerifier::FindLocksAtDexPc(ArtMethod* m, uint32_t dex_pc, } static bool HasMonitorEnterInstructions(const DexFile::CodeItem* const code_item) { - for (const Instruction& inst : code_item->Instructions()) { - if (inst.Opcode() == Instruction::MONITOR_ENTER) { + for (const DexInstructionPcPair& inst : code_item->Instructions()) { + if (inst->Opcode() == Instruction::MONITOR_ENTER) { return true; } } @@ -1018,14 +1018,13 @@ bool MethodVerifier::ComputeWidthsAndCountOps() { default: break; } - GetInstructionFlags(it.GetDexPC(instructions.begin())).SetIsOpcode(); + GetInstructionFlags(it.DexPc()).SetIsOpcode(); } if (it != instructions.end()) { const size_t insns_size = code_item_->insns_size_in_code_units_; Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "code did not end where expected (" - << it.GetDexPC(instructions.begin()) << " vs. " - << insns_size << ")"; + << it.DexPc() << " vs. " << insns_size << ")"; return false; } @@ -1106,10 +1105,9 @@ bool MethodVerifier::VerifyInstructions() { /* Flag the start of the method as a branch target, and a GC point due to stack overflow errors */ GetInstructionFlags(0).SetBranchTarget(); GetInstructionFlags(0).SetCompileTimeInfoPoint(); - IterationRange<DexInstructionIterator> instructions = code_item_->Instructions(); - for (auto inst = instructions.begin(); inst != instructions.end(); ++inst) { - const uint32_t dex_pc = inst.GetDexPC(instructions.begin()); - if (!VerifyInstruction<kAllowRuntimeOnlyInstructions>(&*inst, dex_pc)) { + for (const DexInstructionPcPair& inst : code_item_->Instructions()) { + const uint32_t dex_pc = inst.DexPc(); + if (!VerifyInstruction<kAllowRuntimeOnlyInstructions>(&inst.Inst(), dex_pc)) { DCHECK_NE(failures_.size(), 0U); return false; } @@ -1695,9 +1693,8 @@ void MethodVerifier::Dump(VariableIndentationOutputStream* vios) { vios->Stream() << "Dumping instructions and register lines:\n"; ScopedIndentation indent1(vios); - IterationRange<DexInstructionIterator> instructions = code_item_->Instructions(); - for (auto inst = instructions.begin(); inst != instructions.end(); ++inst) { - const size_t dex_pc = inst.GetDexPC(instructions.begin()); + for (const DexInstructionPcPair& inst : code_item_->Instructions()) { + const size_t dex_pc = inst.DexPc(); RegisterLine* reg_line = reg_table_.GetLine(dex_pc); if (reg_line != nullptr) { vios->Stream() << reg_line->Dump(this) << "\n"; @@ -1963,9 +1960,8 @@ bool MethodVerifier::CodeFlowVerifyMethod() { */ int dead_start = -1; - IterationRange<DexInstructionIterator> instructions = code_item_->Instructions(); - for (auto inst = instructions.begin(); inst != instructions.end(); ++inst) { - const uint32_t insn_idx = inst.GetDexPC(instructions.begin()); + for (const DexInstructionPcPair& inst : code_item_->Instructions()) { + const uint32_t insn_idx = inst.DexPc(); /* * Switch-statement data doesn't get "visited" by scanner. It * may or may not be preceded by a padding NOP (for alignment). @@ -1993,7 +1989,7 @@ bool MethodVerifier::CodeFlowVerifyMethod() { if (dead_start >= 0) { LogVerifyInfo() << "dead code " << reinterpret_cast<void*>(dead_start) - << "-" << reinterpret_cast<void*>(instructions.end().GetDexPC(instructions.begin()) - 1); + << "-" << reinterpret_cast<void*>(code_item_->insns_size_in_code_units_ - 1); } // To dump the state of the verify after a method, do something like: // if (dex_file_->PrettyMethod(dex_method_idx_) == diff --git a/test/706-checker-scheduler/src/Main.java b/test/706-checker-scheduler/src/Main.java index 08a23a7fbc..d21596d4bc 100644 --- a/test/706-checker-scheduler/src/Main.java +++ b/test/706-checker-scheduler/src/Main.java @@ -276,6 +276,83 @@ public class Main { } } + // This case tests a bug found in LSA where LSA doesn't understand IntermediateAddress, + // and incorrectly reported no alias between ArraySet1 and ArrayGet2, + // thus ArrayGet2 is scheduled above ArraySet1 incorrectly. + + /// CHECK-START-ARM64: void Main.CrossOverLoop(int[], int[]) scheduler (before) + /// CHECK: <<ParamA:l\d+>> ParameterValue loop:none + /// CHECK: <<ParamB:l\d+>> ParameterValue loop:none + /// CHECK: <<NullB:l\d+>> NullCheck [<<ParamB>>] loop:none + /// CHECK: <<NullA:l\d+>> NullCheck [<<ParamA>>] loop:none + /// CHECK: Phi loop:<<Loop:B\d+>> outer_loop:none + /// CHECK: <<ArrayGet1:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: <<Addr1:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet1:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArrayGet2:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: <<Addr2:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr2>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + + /// CHECK-START-ARM64: void Main.CrossOverLoop(int[], int[]) scheduler (after) + /// CHECK: <<ParamA:l\d+>> ParameterValue loop:none + /// CHECK: <<ParamB:l\d+>> ParameterValue loop:none + /// CHECK: <<NullB:l\d+>> NullCheck [<<ParamB>>] loop:none + /// CHECK: <<NullA:l\d+>> NullCheck [<<ParamA>>] loop:none + /// CHECK: Phi loop:<<Loop:B\d+>> outer_loop:none + /// CHECK: <<ArrayGet1:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: <<Addr1:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet1:v\d+>> ArraySet [<<Addr1>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArrayGet2:i\d+>> ArrayGet [<<NullB>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: <<Addr2:i\d+>> IntermediateAddress [<<NullA>>,{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: <<ArraySet2:v\d+>> ArraySet [<<Addr2>>,{{i\d+}},{{i\d+}}] loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + private static void CrossOverLoop(int a[], int b[]) { + b[20] = 99; + for (int i = 0; i < a.length; i++) { + a[i] = b[20] - 7; + i++; + a[i] = b[20] - 7; + } + } + + // This test case is similar to above cross over loop, + // but has more complex chains of transforming the original references: + // ParameterValue --> BoundType --> NullCheck --> ArrayGet. + // ParameterValue --> BoundType --> NullCheck --> IntermediateAddress --> ArraySet. + // After using LSA to analyze the orginal references, the scheduler should be able + // to find out that 'a' and 'b' may alias, hence unable to schedule these ArraGet/Set. + + /// CHECK-START-ARM64: void Main.CrossOverLoop2(java.lang.Object, java.lang.Object) scheduler (before) + /// CHECK: Phi loop:<<Loop:B\d+>> outer_loop:none + /// CHECK: ArrayGet loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: ArraySet loop:<<Loop>> outer_loop:none + /// CHECK: ArrayGet loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: ArraySet loop:<<Loop>> outer_loop:none + + /// CHECK-START-ARM64: void Main.CrossOverLoop2(java.lang.Object, java.lang.Object) scheduler (after) + /// CHECK: Phi loop:<<Loop:B\d+>> outer_loop:none + /// CHECK: ArrayGet loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: ArraySet loop:<<Loop>> outer_loop:none + /// CHECK: ArrayGet loop:<<Loop>> outer_loop:none + /// CHECK: Add loop:<<Loop>> outer_loop:none + /// CHECK: ArraySet loop:<<Loop>> outer_loop:none + private static void CrossOverLoop2(Object a, Object b) { + ((int[])b)[20] = 99; + for (int i = 0; i < ((int[])a).length; i++) { + ((int[])a)[i] = ((int[])b)[20] - 7; + i++; + ((int[])a)[i] = ((int[])b)[20] - 7; + } + } + /// CHECK-START-ARM: void Main.accessFields() scheduler (before) /// CHECK: InstanceFieldGet /// CHECK: Add |