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

src/hydrogen.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 822 matching lines @@
     HPhi* phi = phi_list[i];
     if (!phi->is_live()) {
       HBasicBlock* block = phi->block();
       block->RemovePhi(phi);
       block->RecordDeletedPhi(phi->merged_index());
     }
   }
 }
 
 
+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);
   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);
       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;
       }
     }
   }
   return true;
 }
 
 
@@ skipping 1431 matching lines @@
     if (current_block() != NULL) {
       HReturn* instr = new(zone()) HReturn(graph()->GetConstantUndefined());
       current_block()->FinishExit(instr);
       set_current_block(NULL);
     }
   }
 
   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()) {
     Bailout("Unsupported phi-use");
     return NULL;
   }
 
   HInferRepresentation rep(graph());
   rep.Analyze();
 
@@ skipping 4418 matching lines @@
     }
   }
 
 #ifdef DEBUG
   if (graph_ != NULL) graph_->Verify();
   if (allocator_ != NULL) allocator_->Verify();
 #endif
 }
 
 } }  // namespace v8::internal