Revision: 20430
Author: [email protected]
Date: Wed Apr 2 11:30:13 2014 UTC
Log: Check that environments assigned via AssignEnvironment are
actually used.
Removed some temporary marker comments on the way.
[email protected]
Review URL: https://codereview.chromium.org/218403006
http://code.google.com/p/v8/source/detail?r=20430
Modified:
/branches/bleeding_edge/src/arm/lithium-arm.cc
/branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
/branches/bleeding_edge/src/arm64/lithium-arm64.cc
/branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-ia32.cc
/branches/bleeding_edge/src/lithium-codegen.cc
/branches/bleeding_edge/src/lithium-codegen.h
/branches/bleeding_edge/src/lithium.cc
/branches/bleeding_edge/src/lithium.h
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
/branches/bleeding_edge/src/x64/lithium-x64.cc
=======================================
--- /branches/bleeding_edge/src/arm/lithium-arm.cc Tue Apr 1 11:42:42 2014
UTC
+++ /branches/bleeding_edge/src/arm/lithium-arm.cc Wed Apr 2 11:30:13 2014
UTC
@@ -623,6 +623,8 @@
!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 @@
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 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Tue Apr 1
11:42:42 2014 UTC
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Wed Apr 2
11:30:13 2014 UTC
@@ -783,6 +783,7 @@
void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment*
environment,
Safepoint::DeoptMode
mode) {
+ environment->set_has_been_used();
if (!environment->HasBeenRegistered()) {
// Physical stack frame layout:
// -x ............. -4 0 ..................................... y
=======================================
--- /branches/bleeding_edge/src/arm64/lithium-arm64.cc Tue Apr 1 07:21:31
2014 UTC
+++ /branches/bleeding_edge/src/arm64/lithium-arm64.cc Wed Apr 2 11:30:13
2014 UTC
@@ -515,6 +515,8 @@
!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 @@
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* 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 @@
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());
=======================================
--- /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Tue Apr 1
11:42:42 2014 UTC
+++ /branches/bleeding_edge/src/arm64/lithium-codegen-arm64.cc Wed Apr 2
11:30:13 2014 UTC
@@ -376,6 +376,7 @@
void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment*
environment,
Safepoint::DeoptMode
mode) {
+ environment->set_has_been_used();
if (!environment->HasBeenRegistered()) {
int frame_count = 0;
int jsframe_count = 0;
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Tue Apr 1
11:59:24 2014 UTC
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Wed Apr 2
11:30:13 2014 UTC
@@ -1051,6 +1051,7 @@
void LCodeGen::RegisterEnvironmentForDeoptimization(
LEnvironment* environment, Safepoint::DeoptMode mode) {
+ environment->set_has_been_used();
if (!environment->HasBeenRegistered()) {
// Physical stack frame layout:
// -x ............. -4 0 ..................................... y
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Tue Apr 1 11:42:42
2014 UTC
+++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Wed Apr 2 11:30:13
2014 UTC
@@ -688,6 +688,8 @@
!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 @@
? 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 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/lithium-codegen.cc Fri Mar 21 09:28:26 2014
UTC
+++ /branches/bleeding_edge/src/lithium-codegen.cc Wed Apr 2 11:30:13 2014
UTC
@@ -120,6 +120,26 @@
last_lazy_deopt_pc_ = masm()->pc_offset();
return !is_aborted();
}
+
+
+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, ...) {
=======================================
--- /branches/bleeding_edge/src/lithium-codegen.h Wed Feb 19 14:03:48 2014
UTC
+++ /branches/bleeding_edge/src/lithium-codegen.h Wed Apr 2 11:30:13 2014
UTC
@@ -68,6 +68,11 @@
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,
=======================================
--- /branches/bleeding_edge/src/lithium.cc Fri Mar 21 09:28:26 2014 UTC
+++ /branches/bleeding_edge/src/lithium.cc Wed Apr 2 11:30:13 2014 UTC
@@ -432,6 +432,7 @@
MarkEmptyBlocks();
if (generator.GenerateCode()) {
+ generator.CheckEnvironmentUsage();
CodeGenerator::MakeCodePrologue(info(), "optimized");
Code::Flags flags = info()->flags();
Handle<Code> code =
=======================================
--- /branches/bleeding_edge/src/lithium.h Tue Mar 11 02:55:06 2014 UTC
+++ /branches/bleeding_edge/src/lithium.h Wed Apr 2 11:30:13 2014 UTC
@@ -426,7 +426,8 @@
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_; }
@@ -441,6 +442,9 @@
LEnvironment* outer() const { return outer_; }
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,
@@ -541,6 +545,7 @@
LEnvironment* outer_;
HEnterInlined* entry_;
Zone* zone_;
+ bool has_been_used_;
};
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Tue Apr 1
15:32:06 2014 UTC
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Wed Apr 2
11:30:13 2014 UTC
@@ -687,6 +687,7 @@
void LCodeGen::RegisterEnvironmentForDeoptimization(LEnvironment*
environment,
Safepoint::DeoptMode
mode) {
+ environment->set_has_been_used();
if (!environment->HasBeenRegistered()) {
// Physical stack frame layout:
// -x ............. -4 0 ..................................... y
=======================================
--- /branches/bleeding_edge/src/x64/lithium-x64.cc Tue Apr 1 15:32:06 2014
UTC
+++ /branches/bleeding_edge/src/x64/lithium-x64.cc Wed Apr 2 11:30:13 2014
UTC
@@ -652,6 +652,8 @@
!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 @@
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 @@
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;
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
