Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1742253002:
[crankshaft] Inline hasOwnProperty when used in fast-case for-in (Closed)
|
Created:
4 years, 9 months ago by Toon Verwaest Modified:
4 years, 9 months ago Reviewers:
Benedikt Meurer CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
Committed: https://crrev.com/8016a2d53f7d4ea3e35b3a43c7097412e6b38c6a
Cr-Commit-Position: refs/heads/master@{#34405}
Patch Set 1 : #
Total comments: 1
Patch Set 2 : Rebase #Patch Set 3 : Fix: disable if OSR. Otherwise elements of for-in can have more elements in the phi (the OSR entry … #
Messages
Total messages: 31 (16 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Inline hasOwnProperty when used in fast-case for-in BUG= ========== to ========== [crankshaft] Inline hasOwnProperty when used in fast-case for-in BUG= ==========
verwaest@chromium.org changed reviewers: + bmeurer@chromium.org
ptal
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
BUG=
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
==========
(shivering) LGTM https://codereview.chromium.org/1742253002/diff/40001/src/crankshaft/hydrogen.cc File src/crankshaft/hydrogen.cc (right): https://codereview.chromium.org/1742253002/diff/40001/src/crankshaft/hydrogen... src/crankshaft/hydrogen.cc:8765: if (cache->enumerable() != receiver->OperandAt(0)) return false; This subgraph pattern matching in Crankshaft is super scary. It looks OK on the surface, so let's hope that it will not explode too soonish...
We can always revert if it does ...
The CQ bit was checked by verwaest@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1742253002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1742253002/40001
Message was sent while issue was closed.
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
==========
Message was sent while issue was closed.
Committed patchset #1 (id:40001)
Message was sent while issue was closed.
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb Cr-Commit-Position: refs/heads/master@{#34379}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:40001) has been created in https://codereview.chromium.org/1748143004/ by verwaest@chromium.org. The reason for reverting is: ARM64 GCStress failure.
Message was sent while issue was closed.
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
The CQ bit was checked by verwaest@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1742253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1742253002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/16276)
The CQ bit was checked by verwaest@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1742253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1742253002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Still LGTM.
The CQ bit was checked by verwaest@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1742253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1742253002/80001
Message was sent while issue was closed.
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001)
Message was sent while issue was closed.
Description was changed from
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
==========
to
==========
[crankshaft] Inline hasOwnProperty when used in fast-case for-in
e.g.,
for (var k in o) {
if (!o.hasOwnProperty(k)) continue;
...
}
without enumerable properties on the prototype chain of o.
BUG=
Committed: https://crrev.com/dec80752eb344dfeb85588e61ac0afd22b11aadb
Cr-Commit-Position: refs/heads/master@{#34379}
Committed: https://crrev.com/8016a2d53f7d4ea3e35b3a43c7097412e6b38c6a
Cr-Commit-Position: refs/heads/master@{#34405}
==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/8016a2d53f7d4ea3e35b3a43c7097412e6b38c6a Cr-Commit-Position: refs/heads/master@{#34405} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
