Check for phi-uses of arguments object before eliminating dead phi's.

src/hydrogen.cc

Index: src/hydrogen.cc
===================================================================
--- src/hydrogen.cc	(revision 8772)
+++ src/hydrogen.cc	(working copy)
@@ -840,6 +840,19 @@
 }
 
 
+bool HGraph::CheckPhis() {
+  int block_count = blocks_.length();
+  for (int i = 0; i < block_count; ++i) {
+    for (int j = 0; j < blocks_[i]->phis()->length(); ++j) {
+      HPhi* phi = blocks_[i]->phis()->at(j);
+      // We don't support phi uses of arguments for now.
+      if (phi->CheckFlag(HValue::kIsArguments)) return false;
+    }
+  }
+  return true;
+}
+
+
 bool HGraph::CollectPhis() {
   int block_count = blocks_.length();
   phi_list_ = new ZoneList<HPhi*>(block_count);
@@ -847,8 +860,6 @@
     for (int j = 0; j < blocks_[i]->phis()->length(); ++j) {
       HPhi* phi = blocks_[i]->phis()->at(j);
       phi_list_->Add(phi);
-      // We don't support phi uses of arguments for now.
-      if (phi->CheckFlag(HValue::kIsArguments)) return false;
       // Check for the hole value (from an uninitialized const).
       for (int k = 0; k < phi->OperandCount(); k++) {
         if (phi->OperandAt(k) == GetConstantHole()) return false;
@@ -2300,6 +2311,10 @@
   graph()->OrderBlocks();
   graph()->AssignDominators();
   graph()->PropagateDeoptimizingMark();
+  if (!graph()->CheckPhis()) {
+    Bailout("Unsupported phi-use");

[Kevin Millikin (Chromium), 2011/08/02 09:21:25]

1.  There should be a unique bailout reason (to distinguish from the later one),
I suggest "Unsupported phi use of arguments object" here and "Unsupported phi
use of uninitialized constant" later.

2. You could consider moving the calls to Bailout into the CheckPhis/CollectPhis
functions, make them void functions, and call them like:

graph()->CheckPhis();
if (HasStackOverflow()) return NULL;

I'm not sure if I like that better.  What do you think?

[Vyacheslav Egorov (Chromium), 2011/08/02 09:26:23]

1. Done.
2. I don't think HasStackOverflow is pretty enough I will keep bailout here.

On 2011/08/02 09:21:25, Kevin Millikin wrote:

+    return NULL;
+  }
   graph()->EliminateRedundantPhis();
   if (FLAG_eliminate_dead_phis) graph()->EliminateUnreachablePhis();
   if (!graph()->CollectPhis()) {