From a1d8ddfaf09545f99bc326dff97ab604d4574eb6 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Mon, 29 Feb 2016 11:46:58 +0000 Subject: [PATCH] Bug fix for polymorphic inlining. The code used to wrongly propagate try/catch information to new blocks. Since it has the same logic as Hraph::InlineInto, extract the code that updates loop and try/catch information to blocks to a shared method. bug:27330865 bug:27372101 bug:27360329 Change-Id: I4386f724d8d412bde5bcc04fda6955bc3bacf5a9 --- compiler/optimizing/inliner.cc | 33 +++----- compiler/optimizing/nodes.cc | 112 +++++++++++++--------------- compiler/optimizing/nodes.h | 7 ++ test/578-polymorphic-inlining/expected.txt | 0 test/578-polymorphic-inlining/info.txt | 2 + test/578-polymorphic-inlining/src/Main.java | 56 ++++++++++++++ 6 files changed, 125 insertions(+), 85 deletions(-) create mode 100644 test/578-polymorphic-inlining/expected.txt create mode 100644 test/578-polymorphic-inlining/info.txt create mode 100644 test/578-polymorphic-inlining/src/Main.java diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index d55009554..e14eb334d 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -562,31 +562,16 @@ void HInliner::CreateDiamondPatternForPolymorphicInline(HInstruction* compare, graph_->reverse_post_order_[++index] = otherwise; graph_->reverse_post_order_[++index] = merge; - // Set the loop information of the newly created blocks. - HLoopInformation* loop_info = cursor_block->GetLoopInformation(); - if (loop_info != nullptr) { - then->SetLoopInformation(cursor_block->GetLoopInformation()); - merge->SetLoopInformation(cursor_block->GetLoopInformation()); - otherwise->SetLoopInformation(cursor_block->GetLoopInformation()); - for (HLoopInformationOutwardIterator loop_it(*cursor_block); - !loop_it.Done(); - loop_it.Advance()) { - loop_it.Current()->Add(then); - loop_it.Current()->Add(merge); - loop_it.Current()->Add(otherwise); - } - // In case the original invoke location was a back edge, we need to update - // the loop to now have the merge block as a back edge. - if (loop_info->IsBackEdge(*original_invoke_block)) { - loop_info->RemoveBackEdge(original_invoke_block); - loop_info->AddBackEdge(merge); - } - } - // Set the try/catch information of the newly created blocks. - then->SetTryCatchInformation(cursor_block->GetTryCatchInformation()); - merge->SetTryCatchInformation(cursor_block->GetTryCatchInformation()); - otherwise->SetTryCatchInformation(cursor_block->GetTryCatchInformation()); + graph_->UpdateLoopAndTryInformationOfNewBlock( + then, original_invoke_block, /* replace_if_back_edge */ false); + graph_->UpdateLoopAndTryInformationOfNewBlock( + otherwise, original_invoke_block, /* replace_if_back_edge */ false); + + // In case the original invoke location was a back edge, we need to update + // the loop to now have the merge block as a back edge. + graph_->UpdateLoopAndTryInformationOfNewBlock( + merge, original_invoke_block, /* replace_if_back_edge */ true); } bool HInliner::TryInlinePolymorphicCallToSameTarget(HInvoke* invoke_instruction, diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 27a5b97f5..87b9c022d 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -1877,6 +1877,42 @@ void HGraph::DeleteDeadEmptyBlock(HBasicBlock* block) { blocks_[block->GetBlockId()] = nullptr; } +void HGraph::UpdateLoopAndTryInformationOfNewBlock(HBasicBlock* block, + HBasicBlock* reference, + bool replace_if_back_edge) { + if (block->IsLoopHeader()) { + // Clear the information of which blocks are contained in that loop. Since the + // information is stored as a bit vector based on block ids, we have to update + // it, as those block ids were specific to the callee graph and we are now adding + // these blocks to the caller graph. + block->GetLoopInformation()->ClearAllBlocks(); + } + + // If not already in a loop, update the loop information. + if (!block->IsInLoop()) { + block->SetLoopInformation(reference->GetLoopInformation()); + } + + // If the block is in a loop, update all its outward loops. + HLoopInformation* loop_info = block->GetLoopInformation(); + if (loop_info != nullptr) { + for (HLoopInformationOutwardIterator loop_it(*block); + !loop_it.Done(); + loop_it.Advance()) { + loop_it.Current()->Add(block); + } + if (replace_if_back_edge && loop_info->IsBackEdge(*reference)) { + loop_info->ReplaceBackEdge(reference, block); + } + } + + // Copy TryCatchInformation if `reference` is a try block, not if it is a catch block. + TryCatchInformation* try_catch_info = reference->IsTryBlock() + ? reference->GetTryCatchInformation() + : nullptr; + block->SetTryCatchInformation(try_catch_info); +} + HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { DCHECK(HasExitBlock()) << "Unimplemented scenario"; // Update the environments in this graph to have the invoke's environment @@ -1991,10 +2027,6 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { size_t index_of_at = IndexOfElement(outer_graph->reverse_post_order_, at); MakeRoomFor(&outer_graph->reverse_post_order_, blocks_added, index_of_at); - HLoopInformation* loop_info = at->GetLoopInformation(); - // Copy TryCatchInformation if `at` is a try block, not if it is a catch block. - TryCatchInformation* try_catch_info = at->IsTryBlock() ? at->GetTryCatchInformation() : nullptr; - // Do a reverse post order of the blocks in the callee and do (1), (2), (3) // and (4) to the blocks that apply. for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { @@ -2005,23 +2037,7 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { current->SetGraph(outer_graph); outer_graph->AddBlock(current); outer_graph->reverse_post_order_[++index_of_at] = current; - if (!current->IsInLoop()) { - current->SetLoopInformation(loop_info); - } else if (current->IsLoopHeader()) { - // Clear the information of which blocks are contained in that loop. Since the - // information is stored as a bit vector based on block ids, we have to update - // it, as those block ids were specific to the callee graph and we are now adding - // these blocks to the caller graph. - current->GetLoopInformation()->ClearAllBlocks(); - } - if (current->IsInLoop()) { - for (HLoopInformationOutwardIterator loop_it(*current); - !loop_it.Done(); - loop_it.Advance()) { - loop_it.Current()->Add(current); - } - } - current->SetTryCatchInformation(try_catch_info); + UpdateLoopAndTryInformationOfNewBlock(current, at, /* replace_if_back_edge */ false); } } @@ -2029,20 +2045,9 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { to->SetGraph(outer_graph); outer_graph->AddBlock(to); outer_graph->reverse_post_order_[++index_of_at] = to; - if (loop_info != nullptr) { - if (!to->IsInLoop()) { - to->SetLoopInformation(loop_info); - } - for (HLoopInformationOutwardIterator loop_it(*at); !loop_it.Done(); loop_it.Advance()) { - loop_it.Current()->Add(to); - } - if (loop_info->IsBackEdge(*at)) { - // Only `to` can become a back edge, as the inlined blocks - // are predecessors of `to`. - loop_info->ReplaceBackEdge(at, to); - } - } - to->SetTryCatchInformation(try_catch_info); + // Only `to` can become a back edge, as the inlined blocks + // are predecessors of `to`. + UpdateLoopAndTryInformationOfNewBlock(to, at, /* replace_if_back_edge */ true); } // Update the next instruction id of the outer graph, so that instructions @@ -2157,32 +2162,17 @@ void HGraph::TransformLoopHeaderForBCE(HBasicBlock* header) { reverse_post_order_[index_of_header++] = false_block; reverse_post_order_[index_of_header++] = new_pre_header; - // Fix loop information. - HLoopInformation* loop_info = old_pre_header->GetLoopInformation(); - if (loop_info != nullptr) { - if_block->SetLoopInformation(loop_info); - true_block->SetLoopInformation(loop_info); - false_block->SetLoopInformation(loop_info); - new_pre_header->SetLoopInformation(loop_info); - // Add blocks to all enveloping loops. - for (HLoopInformationOutwardIterator loop_it(*old_pre_header); - !loop_it.Done(); - loop_it.Advance()) { - loop_it.Current()->Add(if_block); - loop_it.Current()->Add(true_block); - loop_it.Current()->Add(false_block); - loop_it.Current()->Add(new_pre_header); - } - } - - // Fix try/catch information. - TryCatchInformation* try_catch_info = old_pre_header->IsTryBlock() - ? old_pre_header->GetTryCatchInformation() - : nullptr; - if_block->SetTryCatchInformation(try_catch_info); - true_block->SetTryCatchInformation(try_catch_info); - false_block->SetTryCatchInformation(try_catch_info); - new_pre_header->SetTryCatchInformation(try_catch_info); + // The pre_header can never be a back edge of a loop. + DCHECK((old_pre_header->GetLoopInformation() == nullptr) || + !old_pre_header->GetLoopInformation()->IsBackEdge(*old_pre_header)); + UpdateLoopAndTryInformationOfNewBlock( + if_block, old_pre_header, /* replace_if_back_edge */ false); + UpdateLoopAndTryInformationOfNewBlock( + true_block, old_pre_header, /* replace_if_back_edge */ false); + UpdateLoopAndTryInformationOfNewBlock( + false_block, old_pre_header, /* replace_if_back_edge */ false); + UpdateLoopAndTryInformationOfNewBlock( + new_pre_header, old_pre_header, /* replace_if_back_edge */ false); } static void CheckAgainstUpperBound(ReferenceTypeInfo rti, ReferenceTypeInfo upper_bound_rti) diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 9eddfc7e0..e3dbe1654 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -353,6 +353,13 @@ class HGraph : public ArenaObject { // and removing the invoke instruction. HInstruction* InlineInto(HGraph* outer_graph, HInvoke* invoke); + // Update the loop and try membership of `block`, which was spawned from `reference`. + // In case `reference` is a back edge, `replace_if_back_edge` notifies whether `block` + // should be the new back edge. + void UpdateLoopAndTryInformationOfNewBlock(HBasicBlock* block, + HBasicBlock* reference, + bool replace_if_back_edge); + // Need to add a couple of blocks to test if the loop body is entered and // put deoptimization instructions, etc. void TransformLoopHeaderForBCE(HBasicBlock* header); diff --git a/test/578-polymorphic-inlining/expected.txt b/test/578-polymorphic-inlining/expected.txt new file mode 100644 index 000000000..e69de29bb diff --git a/test/578-polymorphic-inlining/info.txt b/test/578-polymorphic-inlining/info.txt new file mode 100644 index 000000000..77ec49b34 --- /dev/null +++ b/test/578-polymorphic-inlining/info.txt @@ -0,0 +1,2 @@ +Regression test for polymorphic inlining that used to propagate +wrongly the try/catch information of new blocks. diff --git a/test/578-polymorphic-inlining/src/Main.java b/test/578-polymorphic-inlining/src/Main.java new file mode 100644 index 000000000..22d33d083 --- /dev/null +++ b/test/578-polymorphic-inlining/src/Main.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class Main { + public static void main(String[] args) { + for (int i = 0; i < 20000; ++i) { + $noinline$testInTryCatch(new Main(), i); + $noinline$testInTryCatch(new SubMain(), i); + } + } + + public static void $noinline$testInTryCatch(Main m, int i) { + final int value; + try { + throw new Exception(); + } catch (Exception e) { + // The polymorphic inlining of 'willInlineVoid' used to generate an + // incorrect graph, by setting the inlined blocks as catch blocks. + m.willInlineVoid(i); + return; + } + } + + public void willInlineVoid(int i) { + if (i == 0) { + $noinline$foo(); + } else { + $noinline$foo(); + $noinline$foo(); + } + } + + public static void $noinline$foo() { + if (doThrow) throw new Error(""); + } + + public static boolean doThrow; +} + +class SubMain extends Main { + public void willInlineVoid(int i) { + } +} -- 2.11.0