diff options
author | 2016-01-20 21:23:30 +0000 | |
---|---|---|
committer | 2016-01-20 21:23:30 +0000 | |
commit | 451ad8d1be9a1949ea3c3e3a713a9e76198a8b2d (patch) | |
tree | 4a1bb8da95ff9b971f2ef9f3941b645bcaf211d3 | |
parent | 82fc9bb45dbf8ff728122fb7ab72d1eb7b2f4869 (diff) |
Revert "Inline methods with loops."
This reverts commit 82fc9bb45dbf8ff728122fb7ab72d1eb7b2f4869.
Loop inlining exposes issues with BCE.
Bug: 26689526
Change-Id: Id9983d7f9d3c5579d91e56e4699d4d939517b2dc
-rw-r--r-- | compiler/optimizing/inliner.cc | 12 | ||||
-rw-r--r-- | compiler/optimizing/nodes.cc | 27 | ||||
-rw-r--r-- | compiler/optimizing/nodes.h | 4 | ||||
-rw-r--r-- | test/004-StackWalk/src/Main.java | 8 | ||||
-rw-r--r-- | test/482-checker-loop-back-edge-use/src/Main.java | 6 | ||||
-rw-r--r-- | test/510-checker-try-catch/smali/SsaBuilder.smali | 7 | ||||
-rw-r--r-- | test/564-checker-inline-loop/expected.txt | 0 | ||||
-rw-r--r-- | test/564-checker-inline-loop/info.txt | 1 | ||||
-rw-r--r-- | test/564-checker-inline-loop/src/Main.java | 64 |
9 files changed, 16 insertions, 113 deletions
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc index 2e79df1b84..20c4f1f698 100644 --- a/compiler/optimizing/inliner.cc +++ b/compiler/optimizing/inliner.cc @@ -419,10 +419,7 @@ bool HInliner::TryInline(HInvoke* invoke_instruction, ArtMethod* method, bool do size_t inline_max_code_units = compiler_driver_->GetCompilerOptions().GetInlineMaxCodeUnits(); if (code_item->insns_size_in_code_units_ > inline_max_code_units) { VLOG(compiler) << "Method " << PrettyMethod(method) - << " is too big to inline: " - << code_item->insns_size_in_code_units_ - << " > " - << inline_max_code_units; + << " is too big to inline"; return false; } @@ -642,12 +639,9 @@ bool HInliner::TryBuildAndInline(ArtMethod* resolved_method, for (; !it.Done(); it.Advance()) { HBasicBlock* block = it.Current(); - - if (block->IsLoopHeader() && block->GetLoopInformation()->IsIrreducible()) { - // Don't inline methods with irreducible loops, they could prevent some - // optimizations to run. + if (block->IsLoopHeader()) { VLOG(compiler) << "Method " << PrettyMethod(method_index, callee_dex_file) - << " could not be inlined because it contains an irreducible loop"; + << " could not be inlined because it contains a loop"; return false; } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index adf8734214..2eabadf861 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -288,10 +288,9 @@ void HGraph::SimplifyLoop(HBasicBlock* header) { // Make sure the loop has only one pre header. This simplifies SSA building by having // to just look at the pre header to know which locals are initialized at entry of the - // loop. Also, don't allow the entry block to be a pre header: this simplifies inlining - // this graph. + // loop. size_t number_of_incomings = header->GetPredecessors().size() - info->NumberOfBackEdges(); - if (number_of_incomings != 1 || (GetEntryBlock()->GetSingleSuccessor() == header)) { + if (number_of_incomings != 1) { HBasicBlock* pre_header = new (arena_) HBasicBlock(this, header->GetDexPc()); AddBlock(pre_header); pre_header->AddInstruction(new (arena_) HGoto(header->GetDexPc())); @@ -1838,7 +1837,6 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { DCHECK(GetBlocks()[0]->IsEntryBlock()); DCHECK(GetBlocks()[2]->IsExitBlock()); DCHECK(!body->IsExitBlock()); - DCHECK(!body->IsInLoop()); HInstruction* last = body->GetLastInstruction(); invoke->GetBlock()->instructions_.AddAfter(invoke, body->GetInstructions()); @@ -1897,7 +1895,7 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { // Update the meta information surrounding blocks: // (1) the graph they are now in, // (2) the reverse post order of that graph, - // (3) their potential loop information, inner and outer, + // (3) the potential loop information they are now in, // (4) try block membership. // Note that we do not need to update catch phi inputs because they // correspond to the register file of the outer method which the inlinee @@ -1926,24 +1924,15 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { for (HReversePostOrderIterator it(*this); !it.Done(); it.Advance()) { HBasicBlock* current = it.Current(); if (current != exit_block_ && current != entry_block_ && current != first) { + DCHECK(!current->IsInLoop()); DCHECK(current->GetTryCatchInformation() == nullptr); DCHECK(current->GetGraph() == this); current->SetGraph(outer_graph); outer_graph->AddBlock(current); outer_graph->reverse_post_order_[++index_of_at] = current; - if (!current->IsInLoop()) { + if (loop_info != nullptr) { 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()) { + for (HLoopInformationOutwardIterator loop_it(*at); !loop_it.Done(); loop_it.Advance()) { loop_it.Current()->Add(current); } } @@ -1956,9 +1945,7 @@ HInstruction* HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) { outer_graph->AddBlock(to); outer_graph->reverse_post_order_[++index_of_at] = to; if (loop_info != nullptr) { - if (!to->IsInLoop()) { - to->SetLoopInformation(loop_info); - } + to->SetLoopInformation(loop_info); for (HLoopInformationOutwardIterator loop_it(*at); !loop_it.Done(); loop_it.Advance()) { loop_it.Current()->Add(to); } diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 48201e3d23..e222ef7260 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -689,10 +689,6 @@ class HLoopInformation : public ArenaObject<kArenaAllocLoopInfo> { void Add(HBasicBlock* block); void Remove(HBasicBlock* block); - void ClearAllBlocks() { - blocks_.ClearAllBits(); - } - private: // Internal recursive implementation of `Populate`. void PopulateRecursive(HBasicBlock* block); diff --git a/test/004-StackWalk/src/Main.java b/test/004-StackWalk/src/Main.java index 072b1d0c4d..883ce2c9fe 100644 --- a/test/004-StackWalk/src/Main.java +++ b/test/004-StackWalk/src/Main.java @@ -2,14 +2,14 @@ public class Main { public Main() { } - boolean doThrow = false; - int $noinline$f() throws Exception { g(1); g(2); - // This currently defeats inlining of `f`. - if (doThrow) { throw new Error(); } + // This loop currently defeats inlining of `f`. + for (int i = 0; i < 10; i++) { + Thread.sleep(0); + } return 0; } diff --git a/test/482-checker-loop-back-edge-use/src/Main.java b/test/482-checker-loop-back-edge-use/src/Main.java index f8f0aa3f0a..d0b33b9282 100644 --- a/test/482-checker-loop-back-edge-use/src/Main.java +++ b/test/482-checker-loop-back-edge-use/src/Main.java @@ -40,9 +40,6 @@ public class Main { /// CHECK-EVAL: <<GotoLiv2>> + 2 == <<ArgLoopUse2>> public static void loop2(boolean incoming) { - // Add some code at entry to avoid having the entry block be a pre header. - // This avoids having to create a synthesized block. - System.out.println("Enter"); while (true) { System.out.println("foo"); while (incoming) {} @@ -173,9 +170,6 @@ public class Main { /// CHECK-EVAL: <<GotoLiv1>> + 2 == <<ArgLoopUse>> public static void loop9() { - // Add some code at entry to avoid having the entry block be a pre header. - // This avoids having to create a synthesized block. - System.out.println("Enter"); while (Runtime.getRuntime() != null) { // 'incoming' must only have a use in the inner loop. boolean incoming = field; diff --git a/test/510-checker-try-catch/smali/SsaBuilder.smali b/test/510-checker-try-catch/smali/SsaBuilder.smali index a6a5bfebee..710e849c45 100644 --- a/test/510-checker-try-catch/smali/SsaBuilder.smali +++ b/test/510-checker-try-catch/smali/SsaBuilder.smali @@ -21,7 +21,7 @@ ## CHECK-START: int SsaBuilder.testSimplifyCatchBlock(int, int, int) ssa_builder (after) -## CHECK: name "B1" +## CHECK: name "B0" ## CHECK-NEXT: from_bci ## CHECK-NEXT: to_bci ## CHECK-NEXT: predecessors @@ -39,15 +39,12 @@ ## CHECK: name "<<BExtracted>>" ## CHECK-NEXT: from_bci ## CHECK-NEXT: to_bci -## CHECK-NEXT: predecessors "B1" "<<BCatch>>" +## CHECK-NEXT: predecessors "B0" "<<BCatch>>" ## CHECK-NOT: flags "catch_block" ## CHECK: Add .method public static testSimplifyCatchBlock(III)I .registers 4 - # Avoid entry block be a pre header, which leads to - # the cfg simplifier to add a synthesized block. - goto :catch_all :catch_all add-int/2addr p0, p1 diff --git a/test/564-checker-inline-loop/expected.txt b/test/564-checker-inline-loop/expected.txt deleted file mode 100644 index e69de29bb2..0000000000 --- a/test/564-checker-inline-loop/expected.txt +++ /dev/null diff --git a/test/564-checker-inline-loop/info.txt b/test/564-checker-inline-loop/info.txt deleted file mode 100644 index a590bc6f23..0000000000 --- a/test/564-checker-inline-loop/info.txt +++ /dev/null @@ -1 +0,0 @@ -Tests inlining of loops in the optimizing compiler. diff --git a/test/564-checker-inline-loop/src/Main.java b/test/564-checker-inline-loop/src/Main.java deleted file mode 100644 index 6929913864..0000000000 --- a/test/564-checker-inline-loop/src/Main.java +++ /dev/null @@ -1,64 +0,0 @@ -/* - * 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 { - - /// CHECK-START: int Main.inlineLoop() inliner (before) - /// CHECK-DAG: <<Invoke:i\d+>> InvokeStaticOrDirect - /// CHECK-DAG: Return [<<Invoke>>] - - /// CHECK-START: int Main.inlineLoop() inliner (after) - /// CHECK-NOT: InvokeStaticOrDirect - - /// CHECK-START: int Main.inlineLoop() inliner (after) - /// CHECK-DAG: <<Constant:i\d+>> IntConstant 42 - /// CHECK-DAG: Return [<<Constant>>] - - /// CHECK-START: int Main.inlineLoop() licm (after) - /// CHECK: Goto loop:{{B\d+}} - - public static int inlineLoop() { - return loopMethod(); - } - - /// CHECK-START: void Main.inlineWithinLoop() inliner (before) - /// CHECK: InvokeStaticOrDirect - - /// CHECK-START: void Main.inlineWithinLoop() inliner (after) - /// CHECK-NOT: InvokeStaticOrDirect - - /// CHECK-START: void Main.inlineWithinLoop() licm (after) - /// CHECK-DAG: Goto loop:<<OuterLoop:B\d+>> outer_loop:none - /// CHECK-DAG: Goto outer_loop:<<OuterLoop>> - - public static void inlineWithinLoop() { - while (doLoop) { - loopMethod(); - } - } - - public static int loopMethod() { - while (doLoop) {} - return 42; - } - - public static boolean doLoop = false; - - public static void main(String[] args) { - inlineLoop(); - inlineWithinLoop(); - } -} |