Check that environments assigned via AssignEnvironment are actually used.

Removed some temporary marker comments on the way.

R=bmeurer@chromium.org

Review URL: https://codereview.chromium.org/218403006

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20430 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/lithium-arm.cc

@@ -623,6 +623,8 @@ LInstruction* LChunkBuilder::MarkAsCall(LInstruction* instr,
       !hinstr->HasObservableSideEffects();
   if (needs_environment && !instr->HasEnvironment()) {
     instr = AssignEnvironment(instr);
+    // We can't really figure out if the environment is needed or not.
+    instr->environment()->set_has_been_used();
   }
 
   return instr;
@@ -1895,10 +1897,7 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) {
         LOperand* temp2 = FixedTemp(d11);
         LInstruction* result =
             DefineSameAsFirst(new(zone()) LTaggedToI(value, temp1, temp2));
-        if (!val->representation().IsSmi()) {
-          // Note: Only deopts in deferred code.
-          result = AssignEnvironment(result);
-        }
+        if (!val->representation().IsSmi()) result = AssignEnvironment(result);
         return result;
       }
     }
@@ -1993,11 +1992,10 @@ LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) {
     value = UseRegisterAtStart(instr->value());
     if (instr->has_migration_target()) info()->MarkAsDeferredCalling();
   }
-  LCheckMaps* result = new(zone()) LCheckMaps(value);
+  LInstruction* result = new(zone()) LCheckMaps(value);
   if (!instr->CanOmitMapChecks()) {
-    // Note: Only deopts in deferred code.
-    AssignEnvironment(result);
-    if (instr->has_migration_target()) return AssignPointerMap(result);
+    result = AssignEnvironment(result);
+    if (instr->has_migration_target()) result = AssignPointerMap(result);
   }
   return result;
 }

src/arm/lithium-codegen-arm.cc

@@ -783,6 +783,7 @@ void LCodeGen::CallRuntimeFromDeferred(Runtime::FunctionId id,
 
 void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment* environment,
                                                     Safepoint::DeoptMode mode) {
+  environment->set_has_been_used();
   if (!environment->HasBeenRegistered()) {
     // Physical stack frame layout:
     // -x ............. -4  0 ..................................... y

src/arm64/lithium-arm64.cc

@@ -515,6 +515,8 @@ LInstruction* LChunkBuilder::MarkAsCall(LInstruction* instr,
       !hinstr->HasObservableSideEffects();
   if (needs_environment && !instr->HasEnvironment()) {
     instr = AssignEnvironment(instr);
+    // We can't really figure out if the environment is needed or not.
+    instr->environment()->set_has_been_used();
   }
 
   return instr;
@@ -1107,10 +1109,7 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) {
         LOperand* temp2 = instr->CanTruncateToInt32() ? NULL : FixedTemp(d24);
         LInstruction* result =
             DefineAsRegister(new(zone()) LTaggedToI(value, temp1, temp2));
-        if (!val->representation().IsSmi()) {
-          // Note: Only deopts in deferred code.
-          result = AssignEnvironment(result);
-        }
+        if (!val->representation().IsSmi()) result = AssignEnvironment(result);
         return result;
       }
     }
@@ -1194,9 +1193,8 @@ LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) {
   }
   LInstruction* result = new(zone()) LCheckMaps(value, temp);
   if (!instr->CanOmitMapChecks()) {
-    // Note: Only deopts in deferred code.
     result = AssignEnvironment(result);
-    if (instr->has_migration_target()) return AssignPointerMap(result);
+    if (instr->has_migration_target()) result = AssignPointerMap(result);
   }
   return result;
 }
@@ -2432,7 +2430,6 @@ LInstruction* LChunkBuilder::DoUnaryMathOperation(HUnaryMathOperation* instr) {
         LOperand* temp3 = TempRegister();
         LInstruction* result = DefineAsRegister(
             new(zone()) LMathAbsTagged(context, input, temp1, temp2, temp3));
-        // Note: Only deopts in deferred code.
         return AssignEnvironment(AssignPointerMap(result));
       } else {
         LOperand* input = UseRegisterAtStart(instr->value());

src/arm64/lithium-codegen-arm64.cc

@@ -376,6 +376,7 @@ int LCodeGen::DefineDeoptimizationLiteral(Handle<Object> literal) {
 
 void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment* environment,
                                                     Safepoint::DeoptMode mode) {
+  environment->set_has_been_used();
   if (!environment->HasBeenRegistered()) {
     int frame_count = 0;
     int jsframe_count = 0;

src/ia32/lithium-codegen-ia32.cc

@@ -1051,6 +1051,7 @@ void LCodeGen::CallRuntimeFromDeferred(Runtime::FunctionId id,
 
 void LCodeGen::RegisterEnvironmentForDeoptimization(
     LEnvironment* environment, Safepoint::DeoptMode mode) {
+  environment->set_has_been_used();
   if (!environment->HasBeenRegistered()) {
     // Physical stack frame layout:
     // -x ............. -4  0 ..................................... y

src/ia32/lithium-ia32.cc

@@ -688,6 +688,8 @@ LInstruction* LChunkBuilder::MarkAsCall(LInstruction* instr,
       !hinstr->HasObservableSideEffects();
   if (needs_environment && !instr->HasEnvironment()) {
     instr = AssignEnvironment(instr);
+    // We can't really figure out if the environment is needed or not.
+    instr->environment()->set_has_been_used();
   }
 
   return instr;
@@ -1934,10 +1936,7 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) {
                 ? FixedTemp(xmm1) : NULL;
         LInstruction* result =
             DefineSameAsFirst(new(zone()) LTaggedToI(value, xmm_temp));
-        if (!val->representation().IsSmi()) {
-          // Note: Only deopts in deferred code.
-          result = AssignEnvironment(result);
-        }
+        if (!val->representation().IsSmi()) result = AssignEnvironment(result);
         return result;
       }
     }
@@ -2043,11 +2042,10 @@ LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) {
     value = UseRegisterAtStart(instr->value());
     if (instr->has_migration_target()) info()->MarkAsDeferredCalling();
   }
-  LCheckMaps* result = new(zone()) LCheckMaps(value);
+  LInstruction* result = new(zone()) LCheckMaps(value);
   if (!instr->CanOmitMapChecks()) {
-    // Note: Only deopts in deferred code.
-    AssignEnvironment(result);
-    if (instr->has_migration_target()) return AssignPointerMap(result);
+    result = AssignEnvironment(result);
+    if (instr->has_migration_target()) result = AssignPointerMap(result);
   }
   return result;
 }

src/lithium-codegen.cc

@@ -122,6 +122,26 @@ bool LCodeGenBase::GenerateBody() {
 }
 
 
+void LCodeGenBase::CheckEnvironmentUsage() {
+#ifdef DEBUG
+  bool live_block = true;
+  for (int i = 0; i < instructions_->length(); i++) {
+    LInstruction* instr = instructions_->at(i);
+    if (instr->IsLabel()) live_block = !LLabel::cast(instr)->HasReplacement();
+    if (live_block &&
+        instr->hydrogen_value()->block()->IsReachable() &&
+        instr->HasEnvironment() &&
+        !instr->environment()->has_been_used()) {
+      FunctionLiteral* lit = info_->function();
+      V8_Fatal(__FILE__, __LINE__, "unused environment in %s <@%d,#%d> %s\n",
+               lit == NULL ? "<UNKNOWN>" : lit->name()->ToCString().get(),
+               i, instr->hydrogen_value()->id(), instr->Mnemonic());
+    }
+  }
+#endif
+}
+
+
 void LCodeGenBase::Comment(const char* format, ...) {
   if (!FLAG_code_comments) return;
   char buffer[4 * KB];

src/lithium-codegen.h

@@ -68,6 +68,11 @@ class LCodeGenBase BASE_EMBEDDED {
 
   void RegisterWeakObjectsInOptimizedCode(Handle<Code> code);
 
+  // Check that an environment assigned via AssignEnvironment is actually being
+  // used. Redundant assignments keep things alive longer than necessary, and
+  // consequently lead to worse code, so it's important to minimize this.
+  void CheckEnvironmentUsage();
+
  protected:
   enum Status {
     UNUSED,

src/lithium.cc

@@ -432,6 +432,7 @@ Handle<Code> LChunk::Codegen() {
   MarkEmptyBlocks();
 
   if (generator.GenerateCode()) {
+    generator.CheckEnvironmentUsage();
     CodeGenerator::MakeCodePrologue(info(), "optimized");
     Code::Flags flags = info()->flags();
     Handle<Code> code =

src/lithium.h

@@ -426,7 +426,8 @@ class LEnvironment V8_FINAL : public ZoneObject {
         object_mapping_(0, zone),
         outer_(outer),
         entry_(entry),
-        zone_(zone) { }
+        zone_(zone),
+        has_been_used_(false) { }
 
   Handle<JSFunction> closure() const { return closure_; }
   FrameType frame_type() const { return frame_type_; }
@@ -442,6 +443,9 @@ class LEnvironment V8_FINAL : public ZoneObject {
   HEnterInlined* entry() { return entry_; }
   Zone* zone() const { return zone_; }
 
+  bool has_been_used() const { return has_been_used_; }
+  void set_has_been_used() { has_been_used_ = true; }
+
   void AddValue(LOperand* operand,
                 Representation representation,
                 bool is_uint32) {
@@ -541,6 +545,7 @@ class LEnvironment V8_FINAL : public ZoneObject {
   LEnvironment* outer_;
   HEnterInlined* entry_;
   Zone* zone_;
+  bool has_been_used_;
 };
 
 

src/x64/lithium-codegen-x64.cc

@@ -687,6 +687,7 @@ void LCodeGen::CallRuntimeFromDeferred(Runtime::FunctionId id,
 
 void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment* environment,
                                                     Safepoint::DeoptMode mode) {
+  environment->set_has_been_used();
   if (!environment->HasBeenRegistered()) {
     // Physical stack frame layout:
     // -x ............. -4  0 ..................................... y

src/x64/lithium-x64.cc

@@ -652,6 +652,8 @@ LInstruction* LChunkBuilder::MarkAsCall(LInstruction* instr,
       !hinstr->HasObservableSideEffects();
   if (needs_environment && !instr->HasEnvironment()) {
     instr = AssignEnvironment(instr);
+    // We can't really figure out if the environment is needed or not.
+    instr->environment()->set_has_been_used();
   }
 
   return instr;
@@ -1857,10 +1859,7 @@ LInstruction* LChunkBuilder::DoChange(HChange* instr) {
         LOperand* xmm_temp = truncating ? NULL : FixedTemp(xmm1);
         LInstruction* result =
             DefineSameAsFirst(new(zone()) LTaggedToI(value, xmm_temp));
-        if (!val->representation().IsSmi()) {
-          // Note: Only deopts in deferred code.
-          result = AssignEnvironment(result);
-        }
+        if (!val->representation().IsSmi()) result = AssignEnvironment(result);
         return result;
       }
     }
@@ -1955,11 +1954,10 @@ LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) {
     value = UseRegisterAtStart(instr->value());
     if (instr->has_migration_target()) info()->MarkAsDeferredCalling();
   }
-  LCheckMaps* result = new(zone()) LCheckMaps(value);
+  LInstruction* result = new(zone()) LCheckMaps(value);
   if (!instr->CanOmitMapChecks()) {
-    // Note: Only deopts in deferred code.
-    AssignEnvironment(result);
-    if (instr->has_migration_target()) return AssignPointerMap(result);
+    result = AssignEnvironment(result);
+    if (instr->has_migration_target()) result = AssignPointerMap(result);
   }
   return result;
 }