diff options
Diffstat (limited to 'compiler')
26 files changed, 359 insertions, 326 deletions
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc index ae814b4029..977757fd3e 100644 --- a/compiler/dex/verified_method.cc +++ b/compiler/dex/verified_method.cc @@ -40,6 +40,7 @@ namespace art { const VerifiedMethod* VerifiedMethod::Create(verifier::MethodVerifier* method_verifier, bool compile) { std::unique_ptr<VerifiedMethod> verified_method(new VerifiedMethod); + verified_method->has_verification_failures_ = method_verifier->HasFailures(); if (compile) { /* Generate a register map. */ if (!verified_method->GenerateGcMap(method_verifier)) { diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h index 954cbf4b26..437ae52437 100644 --- a/compiler/dex/verified_method.h +++ b/compiler/dex/verified_method.h @@ -70,6 +70,11 @@ class VerifiedMethod { // by using the check-cast elision peephole optimization in the verifier. bool IsSafeCast(uint32_t pc) const; + // Returns true if there were any errors during verification. + bool HasVerificationFailures() const { + return has_verification_failures_; + } + private: VerifiedMethod() = default; @@ -107,6 +112,8 @@ class VerifiedMethod { // dex PC to dex method index or dex field index based on the instruction. DequickenMap dequicken_map_; SafeCastSet safe_cast_set_; + + bool has_verification_failures_; }; } // namespace art diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index ef47377bcc..641d174935 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -2344,6 +2344,31 @@ CompiledMethod* CompilerDriver::GetCompiledMethod(MethodReference ref) const { return it->second; } +bool CompilerDriver::IsMethodVerifiedWithoutFailures(uint32_t method_idx, + uint16_t class_def_idx, + const DexFile& dex_file) const { + const VerifiedMethod* verified_method = GetVerifiedMethod(&dex_file, method_idx); + if (verified_method != nullptr) { + return !verified_method->HasVerificationFailures(); + } + + // If we can't find verification metadata, check if this is a system class (we trust that system + // classes have their methods verified). If it's not, be conservative and assume the method + // has not been verified successfully. + + // TODO: When compiling the boot image it should be safe to assume that everything is verified, + // even if methods are not found in the verification cache. + const char* descriptor = dex_file.GetClassDescriptor(dex_file.GetClassDef(class_def_idx)); + ClassLinker* class_linker = Runtime::Current()->GetClassLinker(); + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + bool is_system_class = class_linker->FindSystemClass(self, descriptor) != nullptr; + if (!is_system_class) { + self->ClearException(); + } + return is_system_class; +} + size_t CompilerDriver::GetNonRelativeLinkerPatchCount() const { MutexLock mu(Thread::Current(), compiled_methods_lock_); return non_relative_linker_patch_count_; diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index f1066a5005..1a4ae13176 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -425,6 +425,12 @@ class CompilerDriver { void RecordClassStatus(ClassReference ref, mirror::Class::Status status) LOCKS_EXCLUDED(compiled_classes_lock_); + // Checks if the specified method has been verified without failures. Returns + // false if the method is not in the verification results (GetVerificationResults). + bool IsMethodVerifiedWithoutFailures(uint32_t method_idx, + uint16_t class_def_idx, + const DexFile& dex_file) const; + SwapVector<uint8_t>* DeduplicateCode(const ArrayRef<const uint8_t>& code); SwapSrcMap* DeduplicateSrcMappingTable(const ArrayRef<SrcMapElem>& src_map); SwapVector<uint8_t>* DeduplicateMappingTable(const ArrayRef<const uint8_t>& code); diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 2420254046..670c897e2d 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -70,6 +70,7 @@ namespace art { // Separate objects into multiple bins to optimize dirty memory use. static constexpr bool kBinObjects = true; +static constexpr bool kComputeEagerResolvedStrings = false; static void CheckNoDexObjectsCallback(Object* obj, void* arg ATTRIBUTE_UNUSED) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { @@ -645,7 +646,11 @@ void ImageWriter::ProcessStrings() { LOG(INFO) << "Total # image strings=" << total_strings << " combined length=" << num_chars << " prefix saved chars=" << prefix_saved_chars; } - ComputeEagerResolvedStrings(); + // Calling this can in theory fill in some resolved strings. However, in practice it seems to + // never resolve any. + if (kComputeEagerResolvedStrings) { + ComputeEagerResolvedStrings(); + } } void ImageWriter::ComputeEagerResolvedStringsCallback(Object* obj, void* arg ATTRIBUTE_UNUSED) { diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 507b3cd021..2ea920310a 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -2673,7 +2673,6 @@ void LocationsBuilderARM::VisitBooleanNot(HBooleanNot* bool_not) { } void InstructionCodeGeneratorARM::VisitBooleanNot(HBooleanNot* bool_not) { - DCHECK_EQ(bool_not->InputAt(0)->GetType(), Primitive::kPrimBoolean); LocationSummary* locations = bool_not->GetLocations(); Location out = locations->Out(); Location in = locations->InAt(0); diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index f6ec729560..efc41e7c06 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -2286,7 +2286,6 @@ void LocationsBuilderARM64::VisitBooleanNot(HBooleanNot* instruction) { } void InstructionCodeGeneratorARM64::VisitBooleanNot(HBooleanNot* instruction) { - DCHECK_EQ(instruction->InputAt(0)->GetType(), Primitive::kPrimBoolean); __ Eor(OutputRegister(instruction), InputRegisterAt(instruction, 0), vixl::Operand(1)); } diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 6c75f344c3..879216d59b 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -2931,7 +2931,6 @@ void LocationsBuilderX86::VisitBooleanNot(HBooleanNot* bool_not) { } void InstructionCodeGeneratorX86::VisitBooleanNot(HBooleanNot* bool_not) { - DCHECK_EQ(bool_not->InputAt(0)->GetType(), Primitive::kPrimBoolean); LocationSummary* locations = bool_not->GetLocations(); Location in = locations->InAt(0); Location out = locations->Out(); @@ -3865,43 +3864,23 @@ X86Assembler* ParallelMoveResolverX86::GetAssembler() const { } void ParallelMoveResolverX86::MoveMemoryToMemory32(int dst, int src) { - ScratchRegisterScope possible_scratch( - this, kNoRegister, codegen_->GetNumberOfCoreRegisters()); - int temp = possible_scratch.GetRegister(); - if (temp == kNoRegister) { - // Use the stack. - __ pushl(Address(ESP, src)); - __ popl(Address(ESP, dst)); - } else { - Register temp_reg = static_cast<Register>(temp); - __ movl(temp_reg, Address(ESP, src)); - __ movl(Address(ESP, dst), temp_reg); - } + ScratchRegisterScope ensure_scratch( + this, kNoRegister, EAX, codegen_->GetNumberOfCoreRegisters()); + Register temp_reg = static_cast<Register>(ensure_scratch.GetRegister()); + int stack_offset = ensure_scratch.IsSpilled() ? kX86WordSize : 0; + __ movl(temp_reg, Address(ESP, src + stack_offset)); + __ movl(Address(ESP, dst + stack_offset), temp_reg); } void ParallelMoveResolverX86::MoveMemoryToMemory64(int dst, int src) { - ScratchRegisterScope possible_scratch( - this, kNoRegister, codegen_->GetNumberOfCoreRegisters()); - int temp = possible_scratch.GetRegister(); - if (temp == kNoRegister) { - // Use the stack instead. - // Push src low word. - __ pushl(Address(ESP, src)); - // Push src high word. Stack offset = 4. - __ pushl(Address(ESP, src + 4 /* offset */ + kX86WordSize /* high */)); - - // Pop into dst high word. Stack offset = 8. - // Pop with ESP address uses the 'after increment' value of ESP. - __ popl(Address(ESP, dst + 4 /* offset */ + kX86WordSize /* high */)); - // Finally dst low word. Stack offset = 4. - __ popl(Address(ESP, dst)); - } else { - Register temp_reg = static_cast<Register>(temp); - __ movl(temp_reg, Address(ESP, src)); - __ movl(Address(ESP, dst), temp_reg); - __ movl(temp_reg, Address(ESP, src + kX86WordSize)); - __ movl(Address(ESP, dst + kX86WordSize), temp_reg); - } + ScratchRegisterScope ensure_scratch( + this, kNoRegister, EAX, codegen_->GetNumberOfCoreRegisters()); + Register temp_reg = static_cast<Register>(ensure_scratch.GetRegister()); + int stack_offset = ensure_scratch.IsSpilled() ? kX86WordSize : 0; + __ movl(temp_reg, Address(ESP, src + stack_offset)); + __ movl(Address(ESP, dst + stack_offset), temp_reg); + __ movl(temp_reg, Address(ESP, src + stack_offset + kX86WordSize)); + __ movl(Address(ESP, dst + stack_offset + kX86WordSize), temp_reg); } void ParallelMoveResolverX86::EmitMove(size_t index) { @@ -3966,18 +3945,10 @@ void ParallelMoveResolverX86::EmitMove(size_t index) { __ xorps(dest, dest); } else { ScratchRegisterScope ensure_scratch( - this, kNoRegister, codegen_->GetNumberOfCoreRegisters()); - int temp_reg = ensure_scratch.GetRegister(); - if (temp_reg == kNoRegister) { - // Avoid spilling/restoring a scratch register by using the stack. - __ pushl(Immediate(value)); - __ movss(dest, Address(ESP, 0)); - __ addl(ESP, Immediate(4)); - } else { - Register temp = static_cast<Register>(temp_reg); - __ movl(temp, Immediate(value)); - __ movd(dest, temp); - } + this, kNoRegister, EAX, codegen_->GetNumberOfCoreRegisters()); + Register temp = static_cast<Register>(ensure_scratch.GetRegister()); + __ movl(temp, Immediate(value)); + __ movd(dest, temp); } } else { DCHECK(destination.IsStackSlot()) << destination; @@ -4026,96 +3997,42 @@ void ParallelMoveResolverX86::EmitMove(size_t index) { } } -void ParallelMoveResolverX86::Exchange(Register reg1, Register reg2) { - // Prefer to avoid xchg as it isn't speedy on smaller processors. - ScratchRegisterScope possible_scratch( - this, reg1, codegen_->GetNumberOfCoreRegisters()); - int temp_reg = possible_scratch.GetRegister(); - if (temp_reg == kNoRegister || temp_reg == reg2) { - __ pushl(reg1); - __ movl(reg1, reg2); - __ popl(reg2); - } else { - Register temp = static_cast<Register>(temp_reg); - __ movl(temp, reg1); - __ movl(reg1, reg2); - __ movl(reg2, temp); - } -} - void ParallelMoveResolverX86::Exchange(Register reg, int mem) { - ScratchRegisterScope possible_scratch( - this, reg, codegen_->GetNumberOfCoreRegisters()); - int temp_reg = possible_scratch.GetRegister(); - if (temp_reg == kNoRegister) { - __ pushl(Address(ESP, mem)); - __ movl(Address(ESP, mem + kX86WordSize), reg); - __ popl(reg); - } else { - Register temp = static_cast<Register>(temp_reg); - __ movl(temp, Address(ESP, mem)); - __ movl(Address(ESP, mem), reg); - __ movl(reg, temp); - } + Register suggested_scratch = reg == EAX ? EBX : EAX; + ScratchRegisterScope ensure_scratch( + this, reg, suggested_scratch, codegen_->GetNumberOfCoreRegisters()); + + int stack_offset = ensure_scratch.IsSpilled() ? kX86WordSize : 0; + __ movl(static_cast<Register>(ensure_scratch.GetRegister()), Address(ESP, mem + stack_offset)); + __ movl(Address(ESP, mem + stack_offset), reg); + __ movl(reg, static_cast<Register>(ensure_scratch.GetRegister())); } void ParallelMoveResolverX86::Exchange32(XmmRegister reg, int mem) { - ScratchRegisterScope possible_scratch( - this, kNoRegister, codegen_->GetNumberOfCoreRegisters()); - int temp_reg = possible_scratch.GetRegister(); - if (temp_reg == kNoRegister) { - __ pushl(Address(ESP, mem)); - __ movss(Address(ESP, mem + kX86WordSize), reg); - __ movss(reg, Address(ESP, 0)); - __ addl(ESP, Immediate(kX86WordSize)); - } else { - Register temp = static_cast<Register>(temp_reg); - __ movl(temp, Address(ESP, mem)); - __ movss(Address(ESP, mem), reg); - __ movd(reg, temp); - } + ScratchRegisterScope ensure_scratch( + this, kNoRegister, EAX, codegen_->GetNumberOfCoreRegisters()); + + Register temp_reg = static_cast<Register>(ensure_scratch.GetRegister()); + int stack_offset = ensure_scratch.IsSpilled() ? kX86WordSize : 0; + __ movl(temp_reg, Address(ESP, mem + stack_offset)); + __ movss(Address(ESP, mem + stack_offset), reg); + __ movd(reg, temp_reg); } void ParallelMoveResolverX86::Exchange(int mem1, int mem2) { - ScratchRegisterScope possible_scratch1( - this, kNoRegister, codegen_->GetNumberOfCoreRegisters()); - int temp_reg1 = possible_scratch1.GetRegister(); - if (temp_reg1 == kNoRegister) { - // No free registers. Use the stack. - __ pushl(Address(ESP, mem1)); - __ pushl(Address(ESP, mem2 + kX86WordSize)); - // Pop with ESP address uses the 'after increment' value of ESP. - __ popl(Address(ESP, mem1 + kX86WordSize)); - __ popl(Address(ESP, mem2)); - } else { - // Got the first one. Try for a second. - ScratchRegisterScope possible_scratch2( - this, temp_reg1, codegen_->GetNumberOfCoreRegisters()); - int temp_reg2 = possible_scratch2.GetRegister(); - if (temp_reg2 == kNoRegister) { - Register temp = static_cast<Register>(temp_reg1); - // Bummer. Only have one free register to use. - // Save mem1 on the stack. - __ pushl(Address(ESP, mem1)); - - // Copy mem2 into mem1. - __ movl(temp, Address(ESP, mem2 + kX86WordSize)); - __ movl(Address(ESP, mem1 + kX86WordSize), temp); - - // Now pop mem1 into mem2. - // Pop with ESP address uses the 'after increment' value of ESP. - __ popl(Address(ESP, mem2)); - } else { - // Great. We have 2 registers to play with. - Register temp1 = static_cast<Register>(temp_reg1); - Register temp2 = static_cast<Register>(temp_reg2); - DCHECK_NE(temp1, temp2); - __ movl(temp1, Address(ESP, mem1)); - __ movl(temp2, Address(ESP, mem2)); - __ movl(Address(ESP, mem2), temp1); - __ movl(Address(ESP, mem1), temp2); - } - } + ScratchRegisterScope ensure_scratch1( + this, kNoRegister, EAX, codegen_->GetNumberOfCoreRegisters()); + + Register suggested_scratch = ensure_scratch1.GetRegister() == EAX ? EBX : EAX; + ScratchRegisterScope ensure_scratch2( + this, ensure_scratch1.GetRegister(), suggested_scratch, codegen_->GetNumberOfCoreRegisters()); + + int stack_offset = ensure_scratch1.IsSpilled() ? kX86WordSize : 0; + stack_offset += ensure_scratch2.IsSpilled() ? kX86WordSize : 0; + __ movl(static_cast<Register>(ensure_scratch1.GetRegister()), Address(ESP, mem1 + stack_offset)); + __ movl(static_cast<Register>(ensure_scratch2.GetRegister()), Address(ESP, mem2 + stack_offset)); + __ movl(Address(ESP, mem2 + stack_offset), static_cast<Register>(ensure_scratch1.GetRegister())); + __ movl(Address(ESP, mem1 + stack_offset), static_cast<Register>(ensure_scratch2.GetRegister())); } void ParallelMoveResolverX86::EmitSwap(size_t index) { @@ -4124,7 +4041,7 @@ void ParallelMoveResolverX86::EmitSwap(size_t index) { Location destination = move->GetDestination(); if (source.IsRegister() && destination.IsRegister()) { - Exchange(destination.AsRegister<Register>(), source.AsRegister<Register>()); + __ xchgl(destination.AsRegister<Register>(), source.AsRegister<Register>()); } else if (source.IsRegister() && destination.IsStackSlot()) { Exchange(source.AsRegister<Register>(), destination.GetStackIndex()); } else if (source.IsStackSlot() && destination.IsRegister()) { diff --git a/compiler/optimizing/code_generator_x86.h b/compiler/optimizing/code_generator_x86.h index 00a4323a54..368ae0fb0e 100644 --- a/compiler/optimizing/code_generator_x86.h +++ b/compiler/optimizing/code_generator_x86.h @@ -106,7 +106,6 @@ class ParallelMoveResolverX86 : public ParallelMoveResolver { X86Assembler* GetAssembler() const; private: - void Exchange(Register reg1, Register Reg2); void Exchange(Register reg, int mem); void Exchange(int mem1, int mem2); void Exchange32(XmmRegister reg, int mem); diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index aa4d7c6edb..a3d3490357 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -2986,7 +2986,6 @@ void LocationsBuilderX86_64::VisitBooleanNot(HBooleanNot* bool_not) { } void InstructionCodeGeneratorX86_64::VisitBooleanNot(HBooleanNot* bool_not) { - DCHECK_EQ(bool_not->InputAt(0)->GetType(), Primitive::kPrimBoolean); LocationSummary* locations = bool_not->GetLocations(); DCHECK_EQ(locations->InAt(0).AsRegister<CpuRegister>().AsRegister(), locations->Out().AsRegister<CpuRegister>().AsRegister()); @@ -3837,27 +3836,15 @@ void ParallelMoveResolverX86_64::Exchange64(CpuRegister reg, int mem) { void ParallelMoveResolverX86_64::Exchange64(int mem1, int mem2) { ScratchRegisterScope ensure_scratch( - this, TMP, codegen_->GetNumberOfCoreRegisters()); - - int temp_reg = ensure_scratch.GetRegister(); - if (temp_reg == kNoRegister) { - // Use the stack as a temporary. - // Save mem1 on the stack. - __ pushq(Address(CpuRegister(RSP), mem1)); - - // Copy mem2 into mem1. - __ movq(CpuRegister(TMP), Address(CpuRegister(RSP), mem2 + kX86_64WordSize)); - __ movq(Address(CpuRegister(RSP), mem1 + kX86_64WordSize), CpuRegister(TMP)); + this, TMP, RAX, codegen_->GetNumberOfCoreRegisters()); - // Now pop mem1 into mem2. - __ popq(Address(CpuRegister(RSP), mem2)); - } else { - CpuRegister temp = CpuRegister(temp_reg); - __ movq(CpuRegister(TMP), Address(CpuRegister(RSP), mem1)); - __ movq(temp, Address(CpuRegister(RSP), mem2)); - __ movq(Address(CpuRegister(RSP), mem2), CpuRegister(TMP)); - __ movq(Address(CpuRegister(RSP), mem1), temp); - } + int stack_offset = ensure_scratch.IsSpilled() ? kX86_64WordSize : 0; + __ movq(CpuRegister(TMP), Address(CpuRegister(RSP), mem1 + stack_offset)); + __ movq(CpuRegister(ensure_scratch.GetRegister()), + Address(CpuRegister(RSP), mem2 + stack_offset)); + __ movq(Address(CpuRegister(RSP), mem2 + stack_offset), CpuRegister(TMP)); + __ movq(Address(CpuRegister(RSP), mem1 + stack_offset), + CpuRegister(ensure_scratch.GetRegister())); } void ParallelMoveResolverX86_64::Exchange32(XmmRegister reg, int mem) { @@ -3866,13 +3853,6 @@ void ParallelMoveResolverX86_64::Exchange32(XmmRegister reg, int mem) { __ movd(reg, CpuRegister(TMP)); } -void ParallelMoveResolverX86_64::Exchange64(CpuRegister reg1, CpuRegister reg2) { - // Prefer to avoid xchg as it isn't speedy on smaller processors. - __ movq(CpuRegister(TMP), reg1); - __ movq(reg1, reg2); - __ movq(reg2, CpuRegister(TMP)); -} - void ParallelMoveResolverX86_64::Exchange64(XmmRegister reg, int mem) { __ movq(CpuRegister(TMP), Address(CpuRegister(RSP), mem)); __ movsd(Address(CpuRegister(RSP), mem), reg); @@ -3885,7 +3865,7 @@ void ParallelMoveResolverX86_64::EmitSwap(size_t index) { Location destination = move->GetDestination(); if (source.IsRegister() && destination.IsRegister()) { - Exchange64(destination.AsRegister<CpuRegister>(), source.AsRegister<CpuRegister>()); + __ xchgq(destination.AsRegister<CpuRegister>(), source.AsRegister<CpuRegister>()); } else if (source.IsRegister() && destination.IsStackSlot()) { Exchange32(source.AsRegister<CpuRegister>(), destination.GetStackIndex()); } else if (source.IsStackSlot() && destination.IsRegister()) { diff --git a/compiler/optimizing/code_generator_x86_64.h b/compiler/optimizing/code_generator_x86_64.h index 61bf6ac71d..b4876ef161 100644 --- a/compiler/optimizing/code_generator_x86_64.h +++ b/compiler/optimizing/code_generator_x86_64.h @@ -118,7 +118,6 @@ class ParallelMoveResolverX86_64 : public ParallelMoveResolver { void Exchange32(CpuRegister reg, int mem); void Exchange32(XmmRegister reg, int mem); void Exchange32(int mem1, int mem2); - void Exchange64(CpuRegister reg1, CpuRegister reg2); void Exchange64(CpuRegister reg, int mem); void Exchange64(XmmRegister reg, int mem); void Exchange64(int mem1, int mem2); diff --git a/compiler/optimizing/dominator_test.cc b/compiler/optimizing/dominator_test.cc index 7623e421fd..61a7697301 100644 --- a/compiler/optimizing/dominator_test.cc +++ b/compiler/optimizing/dominator_test.cc @@ -36,7 +36,13 @@ static void TestCode(const uint16_t* data, const int* blocks, size_t blocks_leng ASSERT_EQ(graph->GetBlocks().Size(), blocks_length); for (size_t i = 0, e = blocks_length; i < e; ++i) { if (blocks[i] == -1) { - ASSERT_EQ(nullptr, graph->GetBlocks().Get(i)->GetDominator()); + if (graph->GetBlocks().Get(i) == nullptr) { + // Dead block. + } else { + // Only the entry block has no dominator. + ASSERT_EQ(nullptr, graph->GetBlocks().Get(i)->GetDominator()); + ASSERT_TRUE(graph->GetBlocks().Get(i)->IsEntryBlock()); + } } else { ASSERT_NE(nullptr, graph->GetBlocks().Get(i)->GetDominator()); ASSERT_EQ(blocks[i], graph->GetBlocks().Get(i)->GetDominator()->GetBlockId()); diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc index 7c3c2bf03d..3a56c6c68f 100644 --- a/compiler/optimizing/graph_checker.cc +++ b/compiler/optimizing/graph_checker.cc @@ -18,6 +18,7 @@ #include <map> #include <string> +#include <sstream> #include "base/bit_vector-inl.h" #include "base/stringprintf.h" @@ -194,6 +195,17 @@ void SSAChecker::VisitBasicBlock(HBasicBlock* block) { } } + // Check Phi uniqueness (no two Phis with the same type refer to the same register). + for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { + HPhi* phi = it.Current()->AsPhi(); + if (phi->GetNextEquivalentPhiWithSameType() != nullptr) { + std::stringstream type_str; + type_str << phi->GetType(); + AddError(StringPrintf("Equivalent phi (%d) found for VReg %d with type: %s", + phi->GetId(), phi->GetRegNumber(), type_str.str().c_str())); + } + } + if (block->IsLoopHeader()) { CheckLoop(block); } @@ -369,26 +381,40 @@ void SSAChecker::VisitPhi(HPhi* phi) { } } -void SSAChecker::VisitIf(HIf* instruction) { - VisitInstruction(instruction); - HInstruction* input = instruction->InputAt(0); +void SSAChecker::HandleBooleanInput(HInstruction* instruction, size_t input_index) { + HInstruction* input = instruction->InputAt(input_index); if (input->IsIntConstant()) { - int value = input->AsIntConstant()->GetValue(); + int32_t value = input->AsIntConstant()->GetValue(); if (value != 0 && value != 1) { AddError(StringPrintf( - "If instruction %d has a non-Boolean constant input " - "whose value is: %d.", + "%s instruction %d has a non-Boolean constant input %d whose value is: %d.", + instruction->DebugName(), instruction->GetId(), + static_cast<int>(input_index), value)); } - } else if (instruction->InputAt(0)->GetType() != Primitive::kPrimBoolean) { + } else if (input->GetType() == Primitive::kPrimInt && input->IsPhi()) { + // TODO: We need a data-flow analysis which determines if the Phi is boolean. + } else if (input->GetType() != Primitive::kPrimBoolean) { AddError(StringPrintf( - "If instruction %d has a non-Boolean input type: %s.", + "%s instruction %d has a non-Boolean input %d whose type is: %s.", + instruction->DebugName(), instruction->GetId(), - Primitive::PrettyDescriptor(instruction->InputAt(0)->GetType()))); + static_cast<int>(input_index), + Primitive::PrettyDescriptor(input->GetType()))); } } +void SSAChecker::VisitIf(HIf* instruction) { + VisitInstruction(instruction); + HandleBooleanInput(instruction, 0); +} + +void SSAChecker::VisitBooleanNot(HBooleanNot* instruction) { + VisitInstruction(instruction); + HandleBooleanInput(instruction, 0); +} + void SSAChecker::VisitCondition(HCondition* op) { VisitInstruction(op); if (op->GetType() != Primitive::kPrimBoolean) { @@ -399,37 +425,23 @@ void SSAChecker::VisitCondition(HCondition* op) { } HInstruction* lhs = op->InputAt(0); HInstruction* rhs = op->InputAt(1); - if (lhs->GetType() == Primitive::kPrimNot) { - if (!op->IsEqual() && !op->IsNotEqual()) { + if (PrimitiveKind(lhs->GetType()) != PrimitiveKind(rhs->GetType())) { + AddError(StringPrintf( + "Condition %s %d has inputs of different types: %s, and %s.", + op->DebugName(), op->GetId(), + Primitive::PrettyDescriptor(lhs->GetType()), + Primitive::PrettyDescriptor(rhs->GetType()))); + } + if (!op->IsEqual() && !op->IsNotEqual()) { + if ((lhs->GetType() == Primitive::kPrimNot)) { AddError(StringPrintf( "Condition %s %d uses an object as left-hand side input.", op->DebugName(), op->GetId())); - } - if (rhs->IsIntConstant() && rhs->AsIntConstant()->GetValue() != 0) { - AddError(StringPrintf( - "Condition %s %d compares an object with a non-zero integer: %d.", - op->DebugName(), op->GetId(), - rhs->AsIntConstant()->GetValue())); - } - } else if (rhs->GetType() == Primitive::kPrimNot) { - if (!op->IsEqual() && !op->IsNotEqual()) { + } else if (rhs->GetType() == Primitive::kPrimNot) { AddError(StringPrintf( "Condition %s %d uses an object as right-hand side input.", op->DebugName(), op->GetId())); } - if (lhs->IsIntConstant() && lhs->AsIntConstant()->GetValue() != 0) { - AddError(StringPrintf( - "Condition %s %d compares a non-zero integer with an object: %d.", - op->DebugName(), op->GetId(), - lhs->AsIntConstant()->GetValue())); - } - } else if (PrimitiveKind(lhs->GetType()) != PrimitiveKind(rhs->GetType())) { - AddError(StringPrintf( - "Condition %s %d has inputs of different types: " - "%s, and %s.", - op->DebugName(), op->GetId(), - Primitive::PrettyDescriptor(lhs->GetType()), - Primitive::PrettyDescriptor(rhs->GetType()))); } } diff --git a/compiler/optimizing/graph_checker.h b/compiler/optimizing/graph_checker.h index 89fea0a07f..24fee373f9 100644 --- a/compiler/optimizing/graph_checker.h +++ b/compiler/optimizing/graph_checker.h @@ -85,6 +85,7 @@ class SSAChecker : public GraphChecker { public: typedef GraphChecker super_type; + // TODO: There's no need to pass a separate allocator as we could get it from the graph. SSAChecker(ArenaAllocator* allocator, HGraph* graph) : GraphChecker(allocator, graph, "art::SSAChecker: ") {} @@ -107,8 +108,11 @@ class SSAChecker : public GraphChecker { void VisitBinaryOperation(HBinaryOperation* op) OVERRIDE; void VisitCondition(HCondition* op) OVERRIDE; void VisitIf(HIf* instruction) OVERRIDE; + void VisitBooleanNot(HBooleanNot* instruction) OVERRIDE; void VisitConstant(HConstant* instruction) OVERRIDE; + void HandleBooleanInput(HInstruction* instruction, size_t input_index); + private: DISALLOW_COPY_AND_ASSIGN(SSAChecker); }; diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 2c17a67867..6d2a8d77e2 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -31,6 +31,8 @@ #include "ssa_phi_elimination.h" #include "scoped_thread_state_change.h" #include "thread.h" +#include "dex/verified_method.h" +#include "dex/verification_results.h" namespace art { @@ -114,9 +116,11 @@ bool HInliner::TryInline(HInvoke* invoke_instruction, return false; } - if (!resolved_method->GetDeclaringClass()->IsVerified()) { + uint16_t class_def_idx = resolved_method->GetDeclaringClass()->GetDexClassDefIndex(); + if (!compiler_driver_->IsMethodVerifiedWithoutFailures( + resolved_method->GetDexMethodIndex(), class_def_idx, *resolved_method->GetDexFile())) { VLOG(compiler) << "Method " << PrettyMethod(method_index, caller_dex_file) - << " is not inlined because its class could not be verified"; + << " couldn't be verified, so it cannot be inlined"; return false; } @@ -258,10 +262,6 @@ bool HInliner::TryBuildAndInline(Handle<mirror::ArtMethod> resolved_method, graph_->SetHasArrayAccesses(true); } - // Now that we have inlined the callee, we need to update the next - // instruction id of the caller, so that new instructions added - // after optimizations get a unique id. - graph_->SetCurrentInstructionId(callee_graph->GetNextInstructionId()); return true; } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index d8a8554610..5fca4fab22 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -51,9 +51,7 @@ void HGraph::RemoveInstructionsAsUsersFromDeadBlocks(const ArenaBitVector& visit for (size_t i = 0; i < blocks_.Size(); ++i) { if (!visited.IsBitSet(i)) { HBasicBlock* block = blocks_.Get(i); - for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { - RemoveAsUser(it.Current()); - } + DCHECK(block->GetPhis().IsEmpty()) << "Phis are not inserted at this stage"; for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) { RemoveAsUser(it.Current()); } @@ -61,19 +59,17 @@ void HGraph::RemoveInstructionsAsUsersFromDeadBlocks(const ArenaBitVector& visit } } -void HGraph::RemoveDeadBlocks(const ArenaBitVector& visited) const { +void HGraph::RemoveDeadBlocks(const ArenaBitVector& visited) { for (size_t i = 0; i < blocks_.Size(); ++i) { if (!visited.IsBitSet(i)) { HBasicBlock* block = blocks_.Get(i); + // We only need to update the successor, which might be live. for (size_t j = 0; j < block->GetSuccessors().Size(); ++j) { block->GetSuccessors().Get(j)->RemovePredecessor(block); } - for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { - block->RemovePhi(it.Current()->AsPhi(), /*ensure_safety=*/ false); - } - for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) { - block->RemoveInstruction(it.Current(), /*ensure_safety=*/ false); - } + // Remove the block from the list of blocks, so that further analyses + // never see it. + blocks_.Put(i, nullptr); } } } @@ -258,6 +254,7 @@ void HGraph::SimplifyCFG() { // (2): Simplify loops by having only one back edge, and one preheader. for (size_t i = 0; i < blocks_.Size(); ++i) { HBasicBlock* block = blocks_.Get(i); + if (block == nullptr) continue; if (block->GetSuccessors().Size() > 1) { for (size_t j = 0; j < block->GetSuccessors().Size(); ++j) { HBasicBlock* successor = block->GetSuccessors().Get(j); @@ -274,8 +271,9 @@ void HGraph::SimplifyCFG() { } bool HGraph::AnalyzeNaturalLoops() const { - for (size_t i = 0; i < blocks_.Size(); ++i) { - HBasicBlock* block = blocks_.Get(i); + // Order does not matter. + for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { + HBasicBlock* block = it.Current(); if (block->IsLoopHeader()) { HLoopInformation* info = block->GetLoopInformation(); if (!info->Populate()) { @@ -964,23 +962,6 @@ static void MakeRoomFor(GrowableArray<HBasicBlock*>* blocks, } void HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { - // Walk over the entry block and: - // - Move constants from the entry block to the outer_graph's entry block, - // - Replace HParameterValue instructions with their real value. - // - Remove suspend checks, that hold an environment. - int parameter_index = 0; - for (HInstructionIterator it(entry_block_->GetInstructions()); !it.Done(); it.Advance()) { - HInstruction* current = it.Current(); - if (current->IsConstant()) { - current->MoveBefore(outer_graph->GetEntryBlock()->GetLastInstruction()); - } else if (current->IsParameterValue()) { - current->ReplaceWith(invoke->InputAt(parameter_index++)); - } else { - DCHECK(current->IsGoto() || current->IsSuspendCheck()); - entry_block_->RemoveInstruction(current); - } - } - if (GetBlocks().Size() == 3) { // Simple case of an entry block, a body block, and an exit block. // Put the body block's instruction into `invoke`'s block. @@ -1106,6 +1087,36 @@ void HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { } } + // Update the next instruction id of the outer graph, so that instructions + // added later get bigger ids than those in the inner graph. + outer_graph->SetCurrentInstructionId(GetNextInstructionId()); + + // Walk over the entry block and: + // - Move constants from the entry block to the outer_graph's entry block, + // - Replace HParameterValue instructions with their real value. + // - Remove suspend checks, that hold an environment. + // We must do this after the other blocks have been inlined, otherwise ids of + // constants could overlap with the inner graph. + int parameter_index = 0; + for (HInstructionIterator it(entry_block_->GetInstructions()); !it.Done(); it.Advance()) { + HInstruction* current = it.Current(); + if (current->IsNullConstant()) { + current->ReplaceWith(outer_graph->GetNullConstant()); + } else if (current->IsIntConstant()) { + current->ReplaceWith(outer_graph->GetIntConstant(current->AsIntConstant()->GetValue())); + } else if (current->IsLongConstant()) { + current->ReplaceWith(outer_graph->GetLongConstant(current->AsLongConstant()->GetValue())); + } else if (current->IsFloatConstant() || current->IsDoubleConstant()) { + // TODO: Don't duplicate floating-point constants. + current->MoveBefore(outer_graph->GetEntryBlock()->GetLastInstruction()); + } else if (current->IsParameterValue()) { + current->ReplaceWith(invoke->InputAt(parameter_index++)); + } else { + DCHECK(current->IsGoto() || current->IsSuspendCheck()); + entry_block_->RemoveInstruction(current); + } + } + // Finally remove the invoke from the caller. invoke->GetBlock()->RemoveInstruction(invoke); } diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 6fb34da191..649038b532 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -253,7 +253,7 @@ class HGraph : public ArenaObject<kArenaAllocMisc> { ArenaBitVector* visited, ArenaBitVector* visiting); void RemoveInstructionsAsUsersFromDeadBlocks(const ArenaBitVector& visited) const; - void RemoveDeadBlocks(const ArenaBitVector& visited) const; + void RemoveDeadBlocks(const ArenaBitVector& visited); template <class InstType, typename ValueType> InstType* CreateConstant(ValueType value, ArenaSafeMap<ValueType, InstType*>* cache); @@ -2752,6 +2752,20 @@ class HPhi : public HInstruction { bool IsDead() const { return !is_live_; } bool IsLive() const { return is_live_; } + // Returns the next equivalent phi (starting from the current one) or null if there is none. + // An equivalent phi is a phi having the same dex register and type. + // It assumes that phis with the same dex register are adjacent. + HPhi* GetNextEquivalentPhiWithSameType() { + HInstruction* next = GetNext(); + while (next != nullptr && next->AsPhi()->GetRegNumber() == reg_number_) { + if (next->GetType() == GetType()) { + return next->AsPhi(); + } + next = next->GetNext(); + } + return nullptr; + } + DECLARE_INSTRUCTION(Phi); protected: diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc index efca1a59ba..a17d6e1822 100644 --- a/compiler/optimizing/optimizing_compiler.cc +++ b/compiler/optimizing/optimizing_compiler.cc @@ -31,6 +31,8 @@ #include "constant_folding.h" #include "dead_code_elimination.h" #include "dex/quick/dex_file_to_method_inliner_map.h" +#include "dex/verified_method.h" +#include "dex/verification_results.h" #include "driver/compiler_driver.h" #include "driver/compiler_options.h" #include "driver/dex_compilation_unit.h" @@ -45,13 +47,13 @@ #include "mirror/art_method-inl.h" #include "nodes.h" #include "prepare_for_register_allocation.h" +#include "reference_type_propagation.h" #include "register_allocator.h" #include "side_effects_analysis.h" #include "ssa_builder.h" #include "ssa_phi_elimination.h" #include "ssa_liveness_analysis.h" #include "utils/assembler.h" -#include "reference_type_propagation.h" namespace art { @@ -592,15 +594,26 @@ CompiledMethod* OptimizingCompiler::Compile(const DexFile::CodeItem* code_item, InvokeType invoke_type, uint16_t class_def_idx, uint32_t method_idx, - jobject class_loader, + jobject jclass_loader, const DexFile& dex_file) const { - CompiledMethod* method = TryCompile(code_item, access_flags, invoke_type, class_def_idx, - method_idx, class_loader, dex_file); + CompilerDriver* compiler_driver = GetCompilerDriver(); + CompiledMethod* method = nullptr; + if (compiler_driver->IsMethodVerifiedWithoutFailures(method_idx, class_def_idx, dex_file)) { + method = TryCompile(code_item, access_flags, invoke_type, class_def_idx, + method_idx, jclass_loader, dex_file); + } else { + if (compiler_driver->GetCompilerOptions().VerifyAtRuntime()) { + compilation_stats_.RecordStat(MethodCompilationStat::kNotCompiledVerifyAtRuntime); + } else { + compilation_stats_.RecordStat(MethodCompilationStat::kNotCompiledClassNotVerified); + } + } + if (method != nullptr) { return method; } method = delegate_->Compile(code_item, access_flags, invoke_type, class_def_idx, method_idx, - class_loader, dex_file); + jclass_loader, dex_file); if (method != nullptr) { compilation_stats_.RecordStat(MethodCompilationStat::kCompiledQuick); diff --git a/compiler/optimizing/optimizing_compiler_stats.h b/compiler/optimizing/optimizing_compiler_stats.h index 4d5b8d0639..d4a936d1c3 100644 --- a/compiler/optimizing/optimizing_compiler_stats.h +++ b/compiler/optimizing/optimizing_compiler_stats.h @@ -45,6 +45,8 @@ enum MethodCompilationStat { kNotCompiledCantAccesType, kNotOptimizedRegisterAllocator, kNotCompiledUnhandledInstruction, + kNotCompiledVerifyAtRuntime, + kNotCompiledClassNotVerified, kRemovedCheckedCast, kRemovedNullCheck, kInstructionSimplifications, @@ -109,6 +111,8 @@ class OptimizingCompilerStats { case kNotCompiledSpaceFilter : return "kNotCompiledSpaceFilter"; case kNotOptimizedRegisterAllocator : return "kNotOptimizedRegisterAllocator"; case kNotCompiledUnhandledInstruction : return "kNotCompiledUnhandledInstruction"; + case kNotCompiledVerifyAtRuntime : return "kNotCompiledVerifyAtRuntime"; + case kNotCompiledClassNotVerified : return "kNotCompiledClassNotVerified"; case kRemovedCheckedCast: return "kRemovedCheckedCast"; case kRemovedNullCheck: return "kRemovedNullCheck"; case kInstructionSimplifications: return "kInstructionSimplifications"; diff --git a/compiler/optimizing/parallel_move_resolver.cc b/compiler/optimizing/parallel_move_resolver.cc index 0c7f0da611..ad92ca59a1 100644 --- a/compiler/optimizing/parallel_move_resolver.cc +++ b/compiler/optimizing/parallel_move_resolver.cc @@ -269,20 +269,6 @@ int ParallelMoveResolver::AllocateScratchRegister(int blocked, } -int ParallelMoveResolver::AllocateScratchRegister(int blocked, - int register_count) { - int scratch = -1; - for (int reg = 0; reg < register_count; ++reg) { - if ((blocked != reg) && IsScratchLocation(Location::RegisterLocation(reg))) { - scratch = reg; - break; - } - } - - return scratch; -} - - ParallelMoveResolver::ScratchRegisterScope::ScratchRegisterScope( ParallelMoveResolver* resolver, int blocked, int if_scratch, int number_of_registers) : resolver_(resolver), @@ -296,16 +282,6 @@ ParallelMoveResolver::ScratchRegisterScope::ScratchRegisterScope( } -ParallelMoveResolver::ScratchRegisterScope::ScratchRegisterScope( - ParallelMoveResolver* resolver, int blocked, int number_of_registers) - : resolver_(resolver), - reg_(kNoRegister), - spilled_(false) { - // We don't want to spill a register if none are free. - reg_ = resolver_->AllocateScratchRegister(blocked, number_of_registers); -} - - ParallelMoveResolver::ScratchRegisterScope::~ScratchRegisterScope() { if (spilled_) { resolver_->RestoreScratch(reg_); diff --git a/compiler/optimizing/parallel_move_resolver.h b/compiler/optimizing/parallel_move_resolver.h index 36ce575609..95f8ad5b74 100644 --- a/compiler/optimizing/parallel_move_resolver.h +++ b/compiler/optimizing/parallel_move_resolver.h @@ -42,15 +42,10 @@ class ParallelMoveResolver : public ValueObject { protected: class ScratchRegisterScope : public ValueObject { public: - // Spill a scratch register if no regs are free. ScratchRegisterScope(ParallelMoveResolver* resolver, int blocked, int if_scratch, int number_of_registers); - // Grab a scratch register only if available. - ScratchRegisterScope(ParallelMoveResolver* resolver, - int blocked, - int number_of_registers); ~ScratchRegisterScope(); int GetRegister() const { return reg_; } @@ -67,8 +62,6 @@ class ParallelMoveResolver : public ValueObject { // Allocate a scratch register for performing a move. The method will try to use // a register that is the destination of a move, but that move has not been emitted yet. int AllocateScratchRegister(int blocked, int if_scratch, int register_count, bool* spilled); - // As above, but return -1 if no free register. - int AllocateScratchRegister(int blocked, int register_count); // Emit a move. virtual void EmitMove(size_t index) = 0; diff --git a/compiler/optimizing/register_allocator.cc b/compiler/optimizing/register_allocator.cc index 2fbd051c03..830f2c26e1 100644 --- a/compiler/optimizing/register_allocator.cc +++ b/compiler/optimizing/register_allocator.cc @@ -1422,7 +1422,6 @@ void RegisterAllocator::ConnectSiblings(LiveInterval* interval) { : Location::StackSlot(interval->GetParent()->GetSpillSlot())); } UsePosition* use = current->GetFirstUse(); - SafepointPosition* safepoint_position = interval->GetFirstSafepoint(); // Walk over all siblings, updating locations of use positions, and // connecting them when they are adjacent. @@ -1473,11 +1472,10 @@ void RegisterAllocator::ConnectSiblings(LiveInterval* interval) { InsertParallelMoveAt(current->GetEnd(), interval->GetDefinedBy(), source, destination); } - for (; safepoint_position != nullptr; safepoint_position = safepoint_position->GetNext()) { - if (!current->Covers(safepoint_position->GetPosition())) { - DCHECK(next_sibling != nullptr); - break; - } + for (SafepointPosition* safepoint_position = current->GetFirstSafepoint(); + safepoint_position != nullptr; + safepoint_position = safepoint_position->GetNext()) { + DCHECK(current->Covers(safepoint_position->GetPosition())); LocationSummary* locations = safepoint_position->GetLocations(); if ((current->GetType() == Primitive::kPrimNot) && current->GetParent()->HasSpillSlot()) { @@ -1523,7 +1521,6 @@ void RegisterAllocator::ConnectSiblings(LiveInterval* interval) { } while (current != nullptr); if (kIsDebugBuild) { - DCHECK(safepoint_position == nullptr); // Following uses can only be environment uses. The location for // these environments will be none. while (use != nullptr) { @@ -1541,35 +1538,14 @@ void RegisterAllocator::ConnectSplitSiblings(LiveInterval* interval, return; } - // Intervals end at the lifetime end of a block. The decrement by one - // ensures the `Cover` call will return true. - size_t from_position = from->GetLifetimeEnd() - 1; - size_t to_position = to->GetLifetimeStart(); - - LiveInterval* destination = nullptr; - LiveInterval* source = nullptr; - - LiveInterval* current = interval; - - // Check the intervals that cover `from` and `to`. - while ((current != nullptr) && (source == nullptr || destination == nullptr)) { - if (current->Covers(from_position)) { - DCHECK(source == nullptr); - source = current; - } - if (current->Covers(to_position)) { - DCHECK(destination == nullptr); - destination = current; - } - - current = current->GetNextSibling(); - } + // Find the intervals that cover `from` and `to`. + LiveInterval* destination = interval->GetSiblingAt(to->GetLifetimeStart()); + LiveInterval* source = interval->GetSiblingAt(from->GetLifetimeEnd() - 1); if (destination == source) { // Interval was not split. return; } - DCHECK(destination != nullptr && source != nullptr); if (!destination->HasRegister()) { diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc index e154ea4ee6..5c3d9bf725 100644 --- a/compiler/optimizing/ssa_builder.cc +++ b/compiler/optimizing/ssa_builder.cc @@ -174,6 +174,54 @@ static bool IsPhiEquivalentOf(HInstruction* instruction, HPhi* phi) { && instruction->AsPhi()->GetRegNumber() == phi->GetRegNumber(); } +void SsaBuilder::FixNullConstantType() { + // The order doesn't matter here. + for (HReversePostOrderIterator itb(*GetGraph()); !itb.Done(); itb.Advance()) { + for (HInstructionIterator it(itb.Current()->GetInstructions()); !it.Done(); it.Advance()) { + HInstruction* equality_instr = it.Current(); + if (!equality_instr->IsEqual() && !equality_instr->IsNotEqual()) { + continue; + } + HInstruction* left = equality_instr->InputAt(0); + HInstruction* right = equality_instr->InputAt(1); + HInstruction* null_instr = nullptr; + + if ((left->GetType() == Primitive::kPrimNot) + && (right->IsNullConstant() || right->IsIntConstant())) { + null_instr = right; + } else if ((right->GetType() == Primitive::kPrimNot) + && (left->IsNullConstant() || left->IsIntConstant())) { + null_instr = left; + } else { + continue; + } + + // If we got here, we are comparing against a reference and the int constant + // should be replaced with a null constant. + if (null_instr->IsIntConstant()) { + DCHECK_EQ(0, null_instr->AsIntConstant()->GetValue()); + equality_instr->ReplaceInput(GetGraph()->GetNullConstant(), null_instr == right ? 1 : 0); + } + } + } +} + +void SsaBuilder::EquivalentPhisCleanup() { + // The order doesn't matter here. + for (HReversePostOrderIterator itb(*GetGraph()); !itb.Done(); itb.Advance()) { + for (HInstructionIterator it(itb.Current()->GetPhis()); !it.Done(); it.Advance()) { + HPhi* phi = it.Current()->AsPhi(); + HPhi* next = phi->GetNextEquivalentPhiWithSameType(); + if (next != nullptr) { + phi->ReplaceWith(next); + DCHECK(next->GetNextEquivalentPhiWithSameType() == nullptr) + << "More then one phi equivalent with type " << phi->GetType() + << " found for phi" << phi->GetId(); + } + } + } +} + void SsaBuilder::BuildSsa() { // 1) Visit in reverse post order. We need to have all predecessors of a block visited // (with the exception of loops) in order to create the right environment for that @@ -209,11 +257,21 @@ void SsaBuilder::BuildSsa() { PrimitiveTypePropagation type_propagation(GetGraph()); type_propagation.Run(); - // 5) Mark dead phis again. Steph 4) may have introduced new phis. + // 5) Fix the type for null constants which are part of an equality comparison. + FixNullConstantType(); + + // 6) When creating equivalent phis we copy the inputs of the original phi which + // may be improperly typed. This will be fixed during the type propagation but + // as a result we may end up with two equivalent phis with the same type for + // the same dex register. This pass cleans them up. + EquivalentPhisCleanup(); + + // 7) Mark dead phis again. Step 4) may have introduced new phis. + // Step 6) might enable the death of new phis. SsaDeadPhiElimination dead_phis(GetGraph()); dead_phis.MarkDeadPhis(); - // 6) Now that the graph is correclty typed, we can get rid of redundant phis. + // 8) Now that the graph is correctly typed, we can get rid of redundant phis. // Note that we cannot do this phase before type propagation, otherwise // we could get rid of phi equivalents, whose presence is a requirement for the // type propagation phase. Note that this is to satisfy statement (a) of the @@ -221,7 +279,7 @@ void SsaBuilder::BuildSsa() { SsaRedundantPhiElimination redundant_phi(GetGraph()); redundant_phi.Run(); - // 7) Make sure environments use the right phi "equivalent": a phi marked dead + // 9) Make sure environments use the right phi "equivalent": a phi marked dead // can have a phi equivalent that is not dead. We must therefore update // all environment uses of the dead phi to use its equivalent. Note that there // can be multiple phis for the same Dex register that are live (for example @@ -248,7 +306,7 @@ void SsaBuilder::BuildSsa() { } } - // 8) Deal with phis to guarantee liveness of phis in case of a debuggable + // 10) Deal with phis to guarantee liveness of phis in case of a debuggable // application. This is for satisfying statement (c) of the SsaBuilder // (see ssa_builder.h). if (GetGraph()->IsDebuggable()) { @@ -256,7 +314,7 @@ void SsaBuilder::BuildSsa() { dead_phi_handler.Run(); } - // 9) Now that the right phis are used for the environments, and we + // 11) Now that the right phis are used for the environments, and we // have potentially revive dead phis in case of a debuggable application, // we can eliminate phis we do not need. Regardless of the debuggable status, // this phase is necessary for statement (b) of the SsaBuilder (see ssa_builder.h), @@ -264,7 +322,7 @@ void SsaBuilder::BuildSsa() { // input types. dead_phis.EliminateDeadPhis(); - // 10) Clear locals. + // 12) Clear locals. for (HInstructionIterator it(GetGraph()->GetEntryBlock()->GetInstructions()); !it.Done(); it.Advance()) { diff --git a/compiler/optimizing/ssa_builder.h b/compiler/optimizing/ssa_builder.h index 569b3e2223..265e95b4ac 100644 --- a/compiler/optimizing/ssa_builder.h +++ b/compiler/optimizing/ssa_builder.h @@ -85,6 +85,9 @@ class SsaBuilder : public HGraphVisitor { static constexpr const char* kSsaBuilderPassName = "ssa_builder"; private: + void FixNullConstantType(); + void EquivalentPhisCleanup(); + static HFloatConstant* GetFloatEquivalent(HIntConstant* constant); static HDoubleConstant* GetDoubleEquivalent(HLongConstant* constant); static HPhi* GetFloatDoubleOrReferenceEquivalentOfPhi(HPhi* phi, Primitive::Type type); diff --git a/compiler/optimizing/ssa_liveness_analysis.cc b/compiler/optimizing/ssa_liveness_analysis.cc index 302df2a1d2..ea0e7c3712 100644 --- a/compiler/optimizing/ssa_liveness_analysis.cc +++ b/compiler/optimizing/ssa_liveness_analysis.cc @@ -402,11 +402,11 @@ int LiveInterval::FindHintAtDefinition() const { for (size_t i = 0, e = defined_by_->InputCount(); i < e; ++i) { HInstruction* input = defined_by_->InputAt(i); size_t end = predecessors.Get(i)->GetLifetimeEnd(); - const LiveInterval& input_interval = input->GetLiveInterval()->GetIntervalAt(end - 1); - if (input_interval.GetEnd() == end) { + LiveInterval* input_interval = input->GetLiveInterval()->GetSiblingAt(end - 1); + if (input_interval->GetEnd() == end) { // If the input dies at the end of the predecessor, we know its register can // be reused. - Location input_location = input_interval.ToLocation(); + Location input_location = input_interval->ToLocation(); if (input_location.IsRegisterKind()) { DCHECK(SameRegisterKind(input_location)); return RegisterOrLowRegister(input_location); @@ -418,12 +418,12 @@ int LiveInterval::FindHintAtDefinition() const { Location out = locations->Out(); if (out.IsUnallocated() && out.GetPolicy() == Location::kSameAsFirstInput) { // Try to use the same register as the first input. - const LiveInterval& input_interval = - GetDefinedBy()->InputAt(0)->GetLiveInterval()->GetIntervalAt(GetStart() - 1); - if (input_interval.GetEnd() == GetStart()) { + LiveInterval* input_interval = + GetDefinedBy()->InputAt(0)->GetLiveInterval()->GetSiblingAt(GetStart() - 1); + if (input_interval->GetEnd() == GetStart()) { // If the input dies at the start of this instruction, we know its register can // be reused. - Location location = input_interval.ToLocation(); + Location location = input_interval->ToLocation(); if (location.IsRegisterKind()) { DCHECK(SameRegisterKind(location)); return RegisterOrLowRegister(location); @@ -487,16 +487,17 @@ Location LiveInterval::ToLocation() const { } Location LiveInterval::GetLocationAt(size_t position) { - return GetIntervalAt(position).ToLocation(); + LiveInterval* sibling = GetSiblingAt(position); + DCHECK(sibling != nullptr); + return sibling->ToLocation(); } -const LiveInterval& LiveInterval::GetIntervalAt(size_t position) { +LiveInterval* LiveInterval::GetSiblingAt(size_t position) { LiveInterval* current = this; - while (!current->Covers(position)) { + while (current != nullptr && !current->IsDefinedAt(position)) { current = current->GetNextSibling(); - DCHECK(current != nullptr); } - return *current; + return current; } } // namespace art diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h index 2b51f949d8..beb4907bc1 100644 --- a/compiler/optimizing/ssa_liveness_analysis.h +++ b/compiler/optimizing/ssa_liveness_analysis.h @@ -372,7 +372,11 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { bool HasRegister() const { return register_ != kNoRegister; } bool IsDeadAt(size_t position) const { - return last_range_->GetEnd() <= position; + return GetEnd() <= position; + } + + bool IsDefinedAt(size_t position) const { + return GetStart() <= position && !IsDeadAt(position); } bool Covers(size_t position) { @@ -492,6 +496,15 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { return defined_by_; } + SafepointPosition* FindSafepointJustBefore(size_t position) const { + for (SafepointPosition* safepoint = first_safepoint_, *previous = nullptr; + safepoint != nullptr; + previous = safepoint, safepoint = safepoint->GetNext()) { + if (safepoint->GetPosition() >= position) return previous; + } + return last_safepoint_; + } + /** * Split this interval at `position`. This interval is changed to: * [start ... position). @@ -504,12 +517,25 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { DCHECK(!is_fixed_); DCHECK_GT(position, GetStart()); - if (last_range_->GetEnd() <= position) { + if (GetEnd() <= position) { // This range dies before `position`, no need to split. return nullptr; } LiveInterval* new_interval = new (allocator_) LiveInterval(allocator_, type_); + SafepointPosition* new_last_safepoint = FindSafepointJustBefore(position); + if (new_last_safepoint == nullptr) { + new_interval->first_safepoint_ = first_safepoint_; + new_interval->last_safepoint_ = last_safepoint_; + first_safepoint_ = last_safepoint_ = nullptr; + } else if (last_safepoint_ != new_last_safepoint) { + new_interval->last_safepoint_ = last_safepoint_; + new_interval->first_safepoint_ = new_last_safepoint->GetNext(); + DCHECK(new_interval->first_safepoint_ != nullptr); + last_safepoint_ = new_last_safepoint; + last_safepoint_->SetNext(nullptr); + } + new_interval->next_sibling_ = next_sibling_; next_sibling_ = new_interval; new_interval->parent_ = parent_; @@ -621,8 +647,8 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { // Returns the location of the interval following its siblings at `position`. Location GetLocationAt(size_t position); - // Finds the interval that covers `position`. - const LiveInterval& GetIntervalAt(size_t position); + // Finds the sibling that is defined at `position`. + LiveInterval* GetSiblingAt(size_t position); // Returns whether `other` and `this` share the same kind of register. bool SameRegisterKind(Location other) const; @@ -748,7 +774,6 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { } SafepointPosition* GetFirstSafepoint() const { - DCHECK_EQ(GetParent(), this) << "Only the first sibling lists safepoints"; return first_safepoint_; } @@ -822,7 +847,7 @@ class LiveInterval : public ArenaObject<kArenaAllocMisc> { LiveRange* first_range_; LiveRange* last_range_; - // Safepoints where this interval is live. Only set in the parent interval. + // Safepoints where this interval is live. SafepointPosition* first_safepoint_; SafepointPosition* last_safepoint_; |