Use stability to only conditionally flush information from the map check table.

Use stability to only conditionally flush information from the map check table.

R=ishell@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21274

[Benedikt Meurer, 2014-05-06 11:33:49]

PTAL

[Igor Sheludko, 2014-05-07 16:53:26]

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
File src/hydrogen-check-elimination.cc (right):

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:6: 
Spurious change?

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:29: CHECKED,
Please add comments about meaning of the states.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:363: entry->state_ = instr->maps_are_stable()
|| entry->check_->maps_are_stable()?

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:567: entry->state_ =
HCheckTableEntry::UNCHECKED_STABLE;
Does it mean that we could end up adding multiple stability check instructions
for the same set of maps?
We could probably fix that by setting CHECKED_STABLE state only for the
stability check instructions and instead of it initially use UNCHECKED_STABLE
for all the other instructions with stable maps. WDYT?

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:737: table->KillUnstableEntries();
I think you can return at this point as all the entries that are killed below
must be unstable anyway.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:758: bool osr_entries_ : 1;
Consider using GVNFlags instead of set of booleans.

[Benedikt Meurer, 2014-05-08 04:45:49]

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
File src/hydrogen-check-elimination.cc (right):

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:6: 
Updated to follow Google C++ Style Guide.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:29: CHECKED,
On 2014/05/07 16:53:26, Igor Sheludko wrote:
> Please add comments about meaning of the states.

Done.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:363: entry->state_ = instr->maps_are_stable()
entry->check_ may be NULL at this point. But entry->state_ is already set
according to the previous check, so it all comes down to setting entry->state_
to CHECKED_STABLE if it was UNCHECKED_STABLE before, or
instr->maps_are_stable().

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:567: entry->state_ =
HCheckTableEntry::UNCHECKED_STABLE;
This is not an issue, because the stability checks are simply markers (i.e. they
do not generate code, they do not add register constraints, they do not
change/depend on flags, etc.), but simply add their maps to a set of maps in the
LChunk.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:737: table->KillUnstableEntries();
No, because we can have stored maps to objects with (UN)CHECKED_STABLE entries,
and this is handled below.

https://codereview.chromium.org/266083007/diff/20001/src/hydrogen-check-elimi...
src/hydrogen-check-elimination.cc:758: bool osr_entries_ : 1;
On 2014/05/07 16:53:26, Igor Sheludko wrote:
> Consider using GVNFlags instead of set of booleans.

Done.

[Igor Sheludko, 2014-05-08 08:53:59]

lgtm

[Igor Sheludko, 2014-05-12 20:05:59]

Committed patchset #3 manually as r21274 (tree was closed).