diff options
author | 2022-09-07 10:31:59 +0000 | |
---|---|---|
committer | 2022-09-08 08:19:12 +0000 | |
commit | 3fcfd7303a7a0683e334d0509b17d65d773b3d71 (patch) | |
tree | 0cc5689bdb0e7b4d944db3f83255cc2dc470482d /compiler/optimizing | |
parent | 567e144f8d0798d539fe8401d0440963311856fd (diff) |
Reland "Add an environment to the beginning of catch blocks"
This reverts commit 37fe26288aaacae0f26873131dd92704796e09ec.
Reason for revert: Pattern match fix
Bug: 227283224
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b
--debuggable 565-checker-condition-liveness (also for --target)
Change-Id: Iaf784c12fb6229df6cfbd9a1b43467f5ed43c4d4
Diffstat (limited to 'compiler/optimizing')
-rw-r--r-- | compiler/optimizing/code_generator.cc | 133 | ||||
-rw-r--r-- | compiler/optimizing/code_generator.h | 7 | ||||
-rw-r--r-- | compiler/optimizing/code_sinking.cc | 15 | ||||
-rw-r--r-- | compiler/optimizing/graph_checker.cc | 36 | ||||
-rw-r--r-- | compiler/optimizing/instruction_builder.cc | 9 |
5 files changed, 138 insertions, 62 deletions
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc index b514f9bf9f..252e756737 100644 --- a/compiler/optimizing/code_generator.cc +++ b/compiler/optimizing/code_generator.cc @@ -413,6 +413,12 @@ void CodeGenerator::Compile(CodeAllocator* allocator) { for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) { HInstruction* current = it.Current(); if (current->HasEnvironment()) { + // Catch StackMaps are dealt with later on in `RecordCatchBlockInfo`. + if (block->IsCatchBlock() && block->GetFirstInstruction() == current) { + DCHECK(current->IsNop()); + continue; + } + // Create stackmap for HNop or any instruction which calls native code. // Note that we need correct mapping for the native PC of the call instruction, // so the runtime's stackmap is not sufficient since it is at PC after the call. @@ -1334,53 +1340,29 @@ void CodeGenerator::RecordCatchBlockInfo() { continue; } - uint32_t dex_pc = block->GetDexPc(); - uint32_t num_vregs = graph_->GetNumberOfVRegs(); - uint32_t native_pc = GetAddressOf(block); + // Get the outer dex_pc + uint32_t outer_dex_pc = block->GetDexPc(); + DCHECK(block->GetFirstInstruction()->IsNop()); + DCHECK(block->GetFirstInstruction()->AsNop()->NeedsEnvironment()); + HEnvironment* const environment = block->GetFirstInstruction()->GetEnvironment(); + DCHECK(environment != nullptr); + HEnvironment* outer_environment = environment; + while (outer_environment->GetParent() != nullptr) { + outer_environment = outer_environment->GetParent(); + } + outer_dex_pc = outer_environment->GetDexPc(); - stack_map_stream->BeginStackMapEntry(dex_pc, + uint32_t native_pc = GetAddressOf(block); + stack_map_stream->BeginStackMapEntry(outer_dex_pc, native_pc, /* register_mask= */ 0, /* sp_mask= */ nullptr, StackMap::Kind::Catch); - HInstruction* current_phi = block->GetFirstPhi(); - for (size_t vreg = 0; vreg < num_vregs; ++vreg) { - while (current_phi != nullptr && current_phi->AsPhi()->GetRegNumber() < vreg) { - HInstruction* next_phi = current_phi->GetNext(); - DCHECK(next_phi == nullptr || - current_phi->AsPhi()->GetRegNumber() <= next_phi->AsPhi()->GetRegNumber()) - << "Phis need to be sorted by vreg number to keep this a linear-time loop."; - current_phi = next_phi; - } - - if (current_phi == nullptr || current_phi->AsPhi()->GetRegNumber() != vreg) { - stack_map_stream->AddDexRegisterEntry(DexRegisterLocation::Kind::kNone, 0); - } else { - Location location = current_phi->GetLocations()->Out(); - switch (location.GetKind()) { - case Location::kStackSlot: { - stack_map_stream->AddDexRegisterEntry( - DexRegisterLocation::Kind::kInStack, location.GetStackIndex()); - break; - } - case Location::kDoubleStackSlot: { - stack_map_stream->AddDexRegisterEntry( - DexRegisterLocation::Kind::kInStack, location.GetStackIndex()); - stack_map_stream->AddDexRegisterEntry( - DexRegisterLocation::Kind::kInStack, location.GetHighStackIndex(kVRegSize)); - ++vreg; - DCHECK_LT(vreg, num_vregs); - break; - } - default: { - // All catch phis must be allocated to a stack slot. - LOG(FATAL) << "Unexpected kind " << location.GetKind(); - UNREACHABLE(); - } - } - } - } + EmitEnvironment(environment, + /* slow_path= */ nullptr, + /* needs_vreg_info= */ true, + /* is_for_catch_handler= */ true); stack_map_stream->EndStackMapEntry(); } @@ -1391,7 +1373,9 @@ void CodeGenerator::AddSlowPath(SlowPathCode* slow_path) { code_generation_data_->AddSlowPath(slow_path); } -void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_path) { +void CodeGenerator::EmitVRegInfo(HEnvironment* environment, + SlowPathCode* slow_path, + bool is_for_catch_handler) { StackMapStream* stack_map_stream = GetStackMapStream(); // Walk over the environment, and record the location of dex registers. for (size_t i = 0, environment_size = environment->Size(); i < environment_size; ++i) { @@ -1446,6 +1430,7 @@ void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_p } case Location::kRegister : { + DCHECK(!is_for_catch_handler); int id = location.reg(); if (slow_path != nullptr && slow_path->IsCoreRegisterSaved(id)) { uint32_t offset = slow_path->GetStackOffsetOfCoreRegister(id); @@ -1467,6 +1452,7 @@ void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_p } case Location::kFpuRegister : { + DCHECK(!is_for_catch_handler); int id = location.reg(); if (slow_path != nullptr && slow_path->IsFpuRegisterSaved(id)) { uint32_t offset = slow_path->GetStackOffsetOfFpuRegister(id); @@ -1488,6 +1474,7 @@ void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_p } case Location::kFpuRegisterPair : { + DCHECK(!is_for_catch_handler); int low = location.low(); int high = location.high(); if (slow_path != nullptr && slow_path->IsFpuRegisterSaved(low)) { @@ -1509,6 +1496,7 @@ void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_p } case Location::kRegisterPair : { + DCHECK(!is_for_catch_handler); int low = location.low(); int high = location.high(); if (slow_path != nullptr && slow_path->IsCoreRegisterSaved(low)) { @@ -1539,9 +1527,54 @@ void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_p } } +void CodeGenerator::EmitVRegInfoOnlyCatchPhis(HEnvironment* environment) { + StackMapStream* stack_map_stream = GetStackMapStream(); + DCHECK(environment->GetHolder()->GetBlock()->IsCatchBlock()); + DCHECK_EQ(environment->GetHolder()->GetBlock()->GetFirstInstruction(), environment->GetHolder()); + HInstruction* current_phi = environment->GetHolder()->GetBlock()->GetFirstPhi(); + for (size_t vreg = 0; vreg < environment->Size(); ++vreg) { + while (current_phi != nullptr && current_phi->AsPhi()->GetRegNumber() < vreg) { + HInstruction* next_phi = current_phi->GetNext(); + DCHECK(next_phi == nullptr || + current_phi->AsPhi()->GetRegNumber() <= next_phi->AsPhi()->GetRegNumber()) + << "Phis need to be sorted by vreg number to keep this a linear-time loop."; + current_phi = next_phi; + } + + if (current_phi == nullptr || current_phi->AsPhi()->GetRegNumber() != vreg) { + stack_map_stream->AddDexRegisterEntry(DexRegisterLocation::Kind::kNone, 0); + } else { + Location location = current_phi->GetLocations()->Out(); + switch (location.GetKind()) { + case Location::kStackSlot: { + stack_map_stream->AddDexRegisterEntry(DexRegisterLocation::Kind::kInStack, + location.GetStackIndex()); + break; + } + case Location::kDoubleStackSlot: { + stack_map_stream->AddDexRegisterEntry(DexRegisterLocation::Kind::kInStack, + location.GetStackIndex()); + stack_map_stream->AddDexRegisterEntry(DexRegisterLocation::Kind::kInStack, + location.GetHighStackIndex(kVRegSize)); + ++vreg; + DCHECK_LT(vreg, environment->Size()); + break; + } + default: { + LOG(FATAL) << "All catch phis must be allocated to a stack slot. Unexpected kind " + << location.GetKind(); + UNREACHABLE(); + } + } + } + } +} + void CodeGenerator::EmitEnvironment(HEnvironment* environment, SlowPathCode* slow_path, - bool needs_vreg_info) { + bool needs_vreg_info, + bool is_for_catch_handler, + bool innermost_environment) { if (environment == nullptr) return; StackMapStream* stack_map_stream = GetStackMapStream(); @@ -1549,7 +1582,11 @@ void CodeGenerator::EmitEnvironment(HEnvironment* environment, if (emit_inline_info) { // We emit the parent environment first. - EmitEnvironment(environment->GetParent(), slow_path, needs_vreg_info); + EmitEnvironment(environment->GetParent(), + slow_path, + needs_vreg_info, + is_for_catch_handler, + /* innermost_environment= */ false); stack_map_stream->BeginInlineInfoEntry(environment->GetMethod(), environment->GetDexPc(), needs_vreg_info ? environment->Size() : 0, @@ -1557,9 +1594,13 @@ void CodeGenerator::EmitEnvironment(HEnvironment* environment, this); } + // If a dex register map is not required we just won't emit it. if (needs_vreg_info) { - // If a dex register map is not required we just won't emit it. - EmitVRegInfo(environment, slow_path); + if (innermost_environment && is_for_catch_handler) { + EmitVRegInfoOnlyCatchPhis(environment); + } else { + EmitVRegInfo(environment, slow_path, is_for_catch_handler); + } } if (emit_inline_info) { diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h index de247a98b9..6b85aaa880 100644 --- a/compiler/optimizing/code_generator.h +++ b/compiler/optimizing/code_generator.h @@ -846,8 +846,11 @@ class CodeGenerator : public DeletableArenaObject<kArenaAllocCodeGenerator> { void BlockIfInRegister(Location location, bool is_out = false) const; void EmitEnvironment(HEnvironment* environment, SlowPathCode* slow_path, - bool needs_vreg_info = true); - void EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_path); + bool needs_vreg_info = true, + bool is_for_catch_handler = false, + bool innermost_environment = true); + void EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_path, bool is_for_catch_handler); + void EmitVRegInfoOnlyCatchPhis(HEnvironment* environment); static void PrepareCriticalNativeArgumentMoves( HInvokeStaticOrDirect* invoke, diff --git a/compiler/optimizing/code_sinking.cc b/compiler/optimizing/code_sinking.cc index 766bb01978..930675b401 100644 --- a/compiler/optimizing/code_sinking.cc +++ b/compiler/optimizing/code_sinking.cc @@ -271,10 +271,21 @@ static HInstruction* FindIdealPosition(HInstruction* instruction, } } for (const HUseListNode<HEnvironment*>& use : instruction->GetEnvUses()) { - HInstruction* user = use.GetUser()->GetHolder(); + HEnvironment* env = use.GetUser(); + HInstruction* user = env->GetHolder(); if (user->GetBlock() == target_block && (insert_pos == nullptr || user->StrictlyDominates(insert_pos))) { - insert_pos = user; + if (target_block->IsCatchBlock() && target_block->GetFirstInstruction() == user) { + // We can sink the instructions past the environment setting Nop. If we do that, we have to + // remove said instruction from the environment. Since we know that we will be sinking the + // instruction to this block and there are no more instructions to consider, we can safely + // remove it from the environment now. + DCHECK(target_block->GetFirstInstruction()->IsNop()); + env->RemoveAsUserOfInput(use.GetIndex()); + env->SetRawEnvAt(use.GetIndex(), /*instruction=*/ nullptr); + } else { + insert_pos = user; + } } } if (insert_pos == nullptr) { diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc index eda6363dda..10711510f2 100644 --- a/compiler/optimizing/graph_checker.cc +++ b/compiler/optimizing/graph_checker.cc @@ -159,6 +159,24 @@ void GraphChecker::VisitBasicBlock(HBasicBlock* block) { } } + // Make sure the first instruction of a catch block is always a Nop that emits an environment. + if (block->IsCatchBlock()) { + if (!block->GetFirstInstruction()->IsNop()) { + AddError(StringPrintf("Block %d doesn't have a Nop as its first instruction.", + current_block_->GetBlockId())); + } else { + HNop* nop = block->GetFirstInstruction()->AsNop(); + if (!nop->NeedsEnvironment()) { + AddError( + StringPrintf("%s:%d is a Nop and the first instruction of block %d, but it doesn't " + "need an environment.", + nop->DebugName(), + nop->GetId(), + current_block_->GetBlockId())); + } + } + } + // Visit this block's list of phis. for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) { HInstruction* current = it.Current(); @@ -342,14 +360,15 @@ void GraphChecker::VisitTryBoundary(HTryBoundary* try_boundary) { } void GraphChecker::VisitLoadException(HLoadException* load) { - // Ensure that LoadException is the first instruction in a catch block. + // Ensure that LoadException is the second instruction in a catch block. The first one should be a + // Nop (checked separately). if (!load->GetBlock()->IsCatchBlock()) { AddError(StringPrintf("%s:%d is in a non-catch block %d.", load->DebugName(), load->GetId(), load->GetBlock()->GetBlockId())); - } else if (load->GetBlock()->GetFirstInstruction() != load) { - AddError(StringPrintf("%s:%d is not the first instruction in catch block %d.", + } else if (load->GetBlock()->GetFirstInstruction()->GetNext() != load) { + AddError(StringPrintf("%s:%d is not the second instruction in catch block %d.", load->DebugName(), load->GetId(), load->GetBlock()->GetBlockId())); @@ -513,17 +532,10 @@ void GraphChecker::VisitInstruction(HInstruction* instruction) { instruction->GetId(), current_block_->GetBlockId())); } else if (instruction->CanThrowIntoCatchBlock()) { - // Find the top-level environment. This corresponds to the environment of - // the catch block since we do not inline methods with try/catch. - HEnvironment* environment = instruction->GetEnvironment(); - while (environment->GetParent() != nullptr) { - environment = environment->GetParent(); - } - - // Find all catch blocks and test that `instruction` has an environment - // value for each one. + // Find all catch blocks and test that `instruction` has an environment value for each one. const HTryBoundary& entry = instruction->GetBlock()->GetTryCatchInformation()->GetTryEntry(); for (HBasicBlock* catch_block : entry.GetExceptionHandlers()) { + const HEnvironment* environment = catch_block->GetFirstInstruction()->GetEnvironment(); for (HInstructionIterator phi_it(catch_block->GetPhis()); !phi_it.Done(); phi_it.Advance()) { HPhi* catch_phi = phi_it.Current()->AsPhi(); if (environment->GetInstructionAt(catch_phi->GetRegNumber()) == nullptr) { diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc index 605427ba11..ba58c8d1fe 100644 --- a/compiler/optimizing/instruction_builder.cc +++ b/compiler/optimizing/instruction_builder.cc @@ -343,6 +343,10 @@ static bool IsBlockPopulated(HBasicBlock* block) { // Suspend checks were inserted into loop headers during building of dominator tree. DCHECK(block->GetFirstInstruction()->IsSuspendCheck()); return block->GetFirstInstruction() != block->GetLastInstruction(); + } else if (block->IsCatchBlock()) { + // Nops were inserted into the beginning of catch blocks. + DCHECK(block->GetFirstInstruction()->IsNop()); + return block->GetFirstInstruction() != block->GetLastInstruction(); } else { return !block->GetInstructions().IsEmpty(); } @@ -387,6 +391,11 @@ bool HInstructionBuilder::Build() { // This is slightly odd because the loop header might not be empty (TryBoundary). // But we're still creating the environment with locals from the top of the block. InsertInstructionAtTop(suspend_check); + } else if (current_block_->IsCatchBlock()) { + // We add an environment emitting instruction at the beginning of each catch block, in order + // to support try catch inlining. + // This is slightly odd because the catch block might not be empty (TryBoundary). + InsertInstructionAtTop(new (allocator_) HNop(block_dex_pc, /* needs_environment= */ true)); } if (block_dex_pc == kNoDexPc || current_block_ != block_builder_->GetBlockAt(block_dex_pc)) { |