Revert "Add an environment to the beginning of catch blocks"
This reverts commit f976aa822dd35496e4e936b5802af0d53d39ac95.
Reason for revert: breaking some tests https://ci.chromium.org/ui/p/art/builders/ci/angler-armv8-ndebug/3244/blamelist
Change-Id: I5c2732e81bef8a7e83b661b1b947d7c079994c1b
diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index 252e756..b514f9b 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -413,12 +413,6 @@
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.
@@ -1340,29 +1334,53 @@
continue;
}
- // 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();
-
+ uint32_t dex_pc = block->GetDexPc();
+ uint32_t num_vregs = graph_->GetNumberOfVRegs();
uint32_t native_pc = GetAddressOf(block);
- stack_map_stream->BeginStackMapEntry(outer_dex_pc,
+
+ stack_map_stream->BeginStackMapEntry(dex_pc,
native_pc,
/* register_mask= */ 0,
/* sp_mask= */ nullptr,
StackMap::Kind::Catch);
- EmitEnvironment(environment,
- /* slow_path= */ nullptr,
- /* needs_vreg_info= */ true,
- /* is_for_catch_handler= */ true);
+ 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();
+ }
+ }
+ }
+ }
stack_map_stream->EndStackMapEntry();
}
@@ -1373,9 +1391,7 @@
code_generation_data_->AddSlowPath(slow_path);
}
-void CodeGenerator::EmitVRegInfo(HEnvironment* environment,
- SlowPathCode* slow_path,
- bool is_for_catch_handler) {
+void CodeGenerator::EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_path) {
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) {
@@ -1430,7 +1446,6 @@
}
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);
@@ -1452,7 +1467,6 @@
}
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);
@@ -1474,7 +1488,6 @@
}
case Location::kFpuRegisterPair : {
- DCHECK(!is_for_catch_handler);
int low = location.low();
int high = location.high();
if (slow_path != nullptr && slow_path->IsFpuRegisterSaved(low)) {
@@ -1496,7 +1509,6 @@
}
case Location::kRegisterPair : {
- DCHECK(!is_for_catch_handler);
int low = location.low();
int high = location.high();
if (slow_path != nullptr && slow_path->IsCoreRegisterSaved(low)) {
@@ -1527,54 +1539,9 @@
}
}
-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 is_for_catch_handler,
- bool innermost_environment) {
+ bool needs_vreg_info) {
if (environment == nullptr) return;
StackMapStream* stack_map_stream = GetStackMapStream();
@@ -1582,11 +1549,7 @@
if (emit_inline_info) {
// We emit the parent environment first.
- EmitEnvironment(environment->GetParent(),
- slow_path,
- needs_vreg_info,
- is_for_catch_handler,
- /* innermost_environment= */ false);
+ EmitEnvironment(environment->GetParent(), slow_path, needs_vreg_info);
stack_map_stream->BeginInlineInfoEntry(environment->GetMethod(),
environment->GetDexPc(),
needs_vreg_info ? environment->Size() : 0,
@@ -1594,13 +1557,9 @@
this);
}
- // If a dex register map is not required we just won't emit it.
if (needs_vreg_info) {
- if (innermost_environment && is_for_catch_handler) {
- EmitVRegInfoOnlyCatchPhis(environment);
- } else {
- EmitVRegInfo(environment, slow_path, is_for_catch_handler);
- }
+ // If a dex register map is not required we just won't emit it.
+ EmitVRegInfo(environment, slow_path);
}
if (emit_inline_info) {
diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h
index 6b85aaa..de247a9 100644
--- a/compiler/optimizing/code_generator.h
+++ b/compiler/optimizing/code_generator.h
@@ -846,11 +846,8 @@
void BlockIfInRegister(Location location, bool is_out = false) const;
void EmitEnvironment(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);
+ bool needs_vreg_info = true);
+ void EmitVRegInfo(HEnvironment* environment, SlowPathCode* slow_path);
static void PrepareCriticalNativeArgumentMoves(
HInvokeStaticOrDirect* invoke,
diff --git a/compiler/optimizing/code_sinking.cc b/compiler/optimizing/code_sinking.cc
index 930675b..766bb01 100644
--- a/compiler/optimizing/code_sinking.cc
+++ b/compiler/optimizing/code_sinking.cc
@@ -271,21 +271,10 @@
}
}
for (const HUseListNode<HEnvironment*>& use : instruction->GetEnvUses()) {
- HEnvironment* env = use.GetUser();
- HInstruction* user = env->GetHolder();
+ HInstruction* user = use.GetUser()->GetHolder();
if (user->GetBlock() == target_block &&
(insert_pos == nullptr || user->StrictlyDominates(insert_pos))) {
- 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;
- }
+ insert_pos = user;
}
}
if (insert_pos == nullptr) {
diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index 1071151..eda6363 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -159,24 +159,6 @@
}
}
- // 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();
@@ -360,15 +342,14 @@
}
void GraphChecker::VisitLoadException(HLoadException* load) {
- // Ensure that LoadException is the second instruction in a catch block. The first one should be a
- // Nop (checked separately).
+ // Ensure that LoadException is the first instruction in a catch block.
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()->GetNext() != load) {
- AddError(StringPrintf("%s:%d is not the second instruction in catch block %d.",
+ } else if (load->GetBlock()->GetFirstInstruction() != load) {
+ AddError(StringPrintf("%s:%d is not the first instruction in catch block %d.",
load->DebugName(),
load->GetId(),
load->GetBlock()->GetBlockId()));
@@ -532,10 +513,17 @@
instruction->GetId(),
current_block_->GetBlockId()));
} else if (instruction->CanThrowIntoCatchBlock()) {
- // Find all catch blocks and test that `instruction` has an environment value for each one.
+ // 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.
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 ba58c8d..605427b 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -343,10 +343,6 @@
// 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();
}
@@ -391,11 +387,6 @@
// 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)) {
diff --git a/test/565-checker-condition-liveness/src/Main.java b/test/565-checker-condition-liveness/src/Main.java
index 350d9ad..17a8613 100644
--- a/test/565-checker-condition-liveness/src/Main.java
+++ b/test/565-checker-condition-liveness/src/Main.java
@@ -33,8 +33,8 @@
/// CHECK-START-{ARM,ARM64}: void Main.testThrowIntoCatchBlock(int, java.lang.Object, int[]) liveness (after)
/// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[23,25]
- /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,23,25,33]
- /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,23,25,33]
+ /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,23,25]
+ /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,23,25]
/// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 env_uses:[23,25]
/// CHECK-DAG: SuspendCheck env:[[_,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:10
/// CHECK-DAG: NullCheck env:[[<<Const1>>,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:20
@@ -43,9 +43,9 @@
/// CHECK-DAG: TryBoundary
/// CHECK-START-{ARM,ARM64}-DEBUGGABLE: void Main.testThrowIntoCatchBlock(int, java.lang.Object, int[]) liveness (after)
- /// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[11,23,25,33]
- /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,23,25,33]
- /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,23,25,33]
+ /// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[11,23,25]
+ /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,23,25]
+ /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,23,25]
/// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 env_uses:[23,25]
/// CHECK-DAG: SuspendCheck env:[[_,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:10
/// CHECK-DAG: NullCheck env:[[<<Const1>>,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:20
@@ -56,8 +56,8 @@
// X86 and X86_64 generate at use site the ArrayLength, meaning only the BoundsCheck will have environment uses.
/// CHECK-START-{X86,X86_64}: void Main.testThrowIntoCatchBlock(int, java.lang.Object, int[]) liveness (after)
/// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[25,25]
- /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,25,25,33]
- /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,25,25,33]
+ /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,25,25]
+ /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,25,25]
/// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 env_uses:[25,25]
/// CHECK-DAG: SuspendCheck env:[[_,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:10
/// CHECK-DAG: NullCheck env:[[<<Const1>>,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:20
@@ -66,9 +66,9 @@
/// CHECK-DAG: TryBoundary
/// CHECK-START-{X86,X86_64}-DEBUGGABLE: void Main.testThrowIntoCatchBlock(int, java.lang.Object, int[]) liveness (after)
- /// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[11,25,25,33]
- /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,25,25,33]
- /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,25,25,33]
+ /// CHECK-DAG: <<IntArg:i\d+>> ParameterValue env_uses:[11,25,25]
+ /// CHECK-DAG: <<RefArg:l\d+>> ParameterValue env_uses:[11,25,25]
+ /// CHECK-DAG: <<Array:l\d+>> ParameterValue env_uses:[11,25,25]
/// CHECK-DAG: <<Const1:i\d+>> IntConstant 1 env_uses:[25,25]
/// CHECK-DAG: SuspendCheck env:[[_,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:10
/// CHECK-DAG: NullCheck env:[[<<Const1>>,<<IntArg>>,<<RefArg>>,<<Array>>]] liveness:20