Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2438553003:
[ic] Support data handlers that represent simple field stores. (Closed)
|
Created:
4 years, 2 months ago by Igor Sheludko Modified:
4 years ago Reviewers:
Jakob Kummerow CC:
v8-reviews_googlegroups.com, Michael Hablich Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[ic] Support data handlers that represent simple field stores.
BUG=
Review-Url: https://codereview.chromium.org/2438553003
Committed: https://crrev.com/c2a5dc81c7a26579a599ddaf31a7cf84706135b9
Cr-Original-Original-Commit-Position: refs/heads/master@{#40503}
Cr-Original-Commit-Position: refs/heads/master@{#40511}
Cr-Commit-Position: refs/heads/master@{#40524}
Patch Set 1 #Patch Set 2 : Fix for a GC stress issues #Patch Set 3 : One more fix for GC stress issues #
Total comments: 2
Messages
Total messages: 51 (35 generated)
Description was changed from ========== [ic] Support data handlers that represent field stores. The handlers are disabled till KeyedStoreIC_Megamorphic is ported to TurboFan. BUG= ========== to ========== [ic] Support data handlers that represent field stores to receiver. The handlers are disabled till KeyedStoreIC_Megamorphic is ported to TurboFan. BUG= ==========
Description was changed from ========== [ic] Support data handlers that represent field stores to receiver. The handlers are disabled till KeyedStoreIC_Megamorphic is ported to TurboFan. BUG= ========== to ========== [ic] Support data handlers that represent simple field stores. The handlers are disabled till KeyedStoreIC_Megamorphic is ported to TurboFan. BUG= ==========
Patchset #1 (id:1) has been deleted
The CQ bit was checked by ishell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [ic] Support data handlers that represent simple field stores. The handlers are disabled till KeyedStoreIC_Megamorphic is ported to TurboFan. BUG= ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
ishell@chromium.org changed reviewers: + jkummerow@chromium.org
PTAL&CQ
The CQ bit was checked by jkummerow@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001)
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:20001) has been created in https://chromiumcodereview.appspot.com/2439053002/ by jgruber@chromium.org. The reason for reverting is: http://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/....
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= ==========
The CQ bit was checked by ishell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ==========
The CQ bit was unchecked by ishell@chromium.org
The CQ bit was checked by ishell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jkummerow@chromium.org Link to the patchset: https://codereview.chromium.org/2438553003/#ps40001 (title: "Fix for a GC stress issues")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:40001) has been created in https://codereview.chromium.org/2442523003/ by ishell@chromium.org. The reason for reverting is: http://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/... "V8 Mac GC Stress".
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ==========
The CQ bit was checked by ishell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I should have not used the smi store handler for the cases that are not supported by StoreFieldStub (in particular, store to heap object field when a field type check is required). PTAL again
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Commit-Position: refs/heads/master@{#40503} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by ishell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by jkummerow@chromium.org
lgtm https://codereview.chromium.org/2438553003/diff/60001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/2438553003/diff/60001/src/ic/ic.cc#newcode1041 src/ic/ic.cc:1041: handler = Smi::FromInt(1); Come to think of it: why is this line not "return;"? What's the advantage of putting dummy values into the cache?
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Original-Commit-Position: refs/heads/master@{#40503} Cr-Original-Commit-Position: refs/heads/master@{#40511} Cr-Commit-Position: refs/heads/master@{#40524} ==========
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Cr-Original-Original-Commit-Position: refs/heads/master@{#40503} Cr-Original-Commit-Position: refs/heads/master@{#40511} Cr-Commit-Position: refs/heads/master@{#40524} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Committed: https://crrev.com/1f697f4231e772df28eb46c39a972f4f6b7e1672 Cr-Commit-Position: refs/heads/master@{#40503} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/1f697f4231e772df28eb46c39a972f4f6b7e1672 Cr-Commit-Position: refs/heads/master@{#40503}
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Committed: https://crrev.com/1f697f4231e772df28eb46c39a972f4f6b7e1672 Cr-Commit-Position: refs/heads/master@{#40503} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Committed: https://crrev.com/d2557f2e9da4beb904ad07f6b125d853b6d7476d Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d2557f2e9da4beb904ad07f6b125d853b6d7476d Cr-Commit-Position: refs/heads/master@{#40511}
Message was sent while issue was closed.
Description was changed from ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Committed: https://crrev.com/d2557f2e9da4beb904ad07f6b125d853b6d7476d Cr-Original-Commit-Position: refs/heads/master@{#40503} Cr-Commit-Position: refs/heads/master@{#40511} ========== to ========== [ic] Support data handlers that represent simple field stores. BUG= Review-Url: https://codereview.chromium.org/2438553003 Committed: https://crrev.com/c2a5dc81c7a26579a599ddaf31a7cf84706135b9 Cr-Original-Original-Commit-Position: refs/heads/master@{#40503} Cr-Original-Commit-Position: refs/heads/master@{#40511} Cr-Commit-Position: refs/heads/master@{#40524} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c2a5dc81c7a26579a599ddaf31a7cf84706135b9 Cr-Commit-Position: refs/heads/master@{#40524}
Message was sent while issue was closed.
https://codereview.chromium.org/2438553003/diff/60001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/2438553003/diff/60001/src/ic/ic.cc#newcode1041 src/ic/ic.cc:1041: handler = Smi::FromInt(1); On 2016/10/24 09:56:19, Jakob Kummerow wrote: > Come to think of it: why is this line not "return;"? What's the advantage of > putting dummy values into the cache? Actually, we should have not even got here with such a map. It should be filtered out with all the other dead maps. I'll address that in next CL. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
