OSDN Git Service

Bug fix for polymorphic inlining.
authorNicolas Geoffray <ngeoffray@google.com>
Mon, 29 Feb 2016 11:46:58 +0000 (11:46 +0000)
committerNicolas Geoffray <ngeoffray@google.com>
Mon, 29 Feb 2016 14:48:10 +0000 (14:48 +0000)
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

(cherry picked from commit a1d8ddfaf09545f99bc326dff97ab604d4574eb6)

Change-Id: Ice0373ec0a1c24d78121634a377f6f502e814cfb

compiler/optimizing/inliner.cc
compiler/optimizing/nodes.cc
compiler/optimizing/nodes.h
test/578-polymorphic-inlining/expected.txt [new file with mode: 0644]
test/578-polymorphic-inlining/info.txt [new file with mode: 0644]
test/578-polymorphic-inlining/src/Main.java [new file with mode: 0644]

index 02a1acc..4d56a41 100644 (file)
@@ -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,
index f36dc6e..d6ab003 100644 (file)
@@ -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)
index 399afab..dcbfbe0 100644 (file)
@@ -351,6 +351,13 @@ class HGraph : public ArenaObject<kArenaAllocGraph> {
   // 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 (file)
index 0000000..e69de29
diff --git a/test/578-polymorphic-inlining/info.txt b/test/578-polymorphic-inlining/info.txt
new file mode 100644 (file)
index 0000000..77ec49b
--- /dev/null
@@ -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 (file)
index 0000000..22d33d0
--- /dev/null
@@ -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) {
+  }
+}