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

diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index 38acd48f3c7b89f3d6d3c819689531a745fc7097..97722ccfa24876d8671978cbabfc49f5ce8fab64 100644 (file)
--- a/src/arm/lithium-arm.cc
+++ b/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;
 }

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 63e60ba66b00a11c106d4dee938fcdf0c316e07c..7b3788e71006efb8ba5ff9d2e80bb1c751d13d0a 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/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

diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index 1888affd683f74e78ebea1c6f441c0f7585c063d..d58e3cc9916bf01890c550f2b581ae158f17c71c 100644 (file)
--- a/src/arm64/lithium-arm64.cc
+++ b/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());

diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc
index 9b8156e754e05d6bfb8dbf13f4e2cd91fcdf34c2..48ea7796773e0b3cb4f5638b597abd967d29f56f 100644 (file)
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/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;

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index 6c934b7ce9a7654a3b8fd6faa4238a2481f899dd..47cb2dcbf06f1cbd2b716ff7b67cb00eefc008bc 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/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

diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index c230deba2882abd2e16e65b9d5a76c19b5594628..66c79d1d587c2cf1857789728993c87e2cc97f43 100644 (file)
--- a/src/ia32/lithium-ia32.cc
+++ b/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;
 }

diff --git a/src/lithium-codegen.cc b/src/lithium-codegen.cc
index be0ff8371afa51ec664d61a3056d2da13ae7ff79..93ae3101a7779e50b21fe4e542334974bc75758e 100644 (file)
--- a/src/lithium-codegen.cc
+++ b/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];

diff --git a/src/lithium-codegen.h b/src/lithium-codegen.h
index 3e8d471ea78deb92f7f5647dd59cebd116f5ffd6..9f06781b852a0800b72602489a475bb2333eac58 100644 (file)
--- a/src/lithium-codegen.h
+++ b/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,

diff --git a/src/lithium.cc b/src/lithium.cc
index 8753ff14aa4f0b0cfbd1fe4b4cd6b2b151da2a20..48567930abc1c3bec19418e4e974a7fc335497d5 100644 (file)
--- a/src/lithium.cc
+++ b/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 =

diff --git a/src/lithium.h b/src/lithium.h
index 8ae5b879dc7bbe6b6f53388b8efec5011d1a89d0..161da8f0a3b6e8ecc0ee8f3cf36add6103db63a1 100644 (file)
--- a/src/lithium.h
+++ b/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_;
 };
 
 

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index a64d3e223fae20d9f17dbb74b1580d58ed847ed3..a8536a1841f379a5078e5fc79716f16e9564024f 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/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

diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index d1c77311ecf9da440af7748ffbf7567f6e5c78c2..90f9a1a236f72eb1b23dac9fcea4262acb986b43 100644 (file)
--- a/src/x64/lithium-x64.cc
+++ b/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;
 }