Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(107)

Issue 2406843002: [Interpreter] Collect feedback about Oddballs in Subtract Stub. (Closed)

Created:
4 years, 2 months ago by mythria
Modified:
4 years, 2 months ago
CC:
v8-reviews_googlegroups.com, jochen (gone - plz use gerrit)
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[Interpreter] Collect feedback about Oddballs in Subtract Stub. Also include feedback about Oddballs when collecting the type feedback. For now, Number and NumberOrOddball are collected separately because crankshaft does not handle NumberOrOddballs consistently. This should change once we fix crankshaft. BUG=v8:4280, v8:5400 LOG=N Committed: https://crrev.com/d5d283b27d406486e2f8d7b44c6d2b3db4f98458 Committed: https://crrev.com/65716011492fce55475f6e57fc3e8d588afede88 Cr-Original-Commit-Position: refs/heads/master@{#40124} Cr-Commit-Position: refs/heads/master@{#40170}

Patch Set 1 #

Total comments: 16

Patch Set 2 : Addressed comments from Leszek. #

Patch Set 3 : Trying with a landmine. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+85 lines, -20 lines) Patch
M gypfiles/get_landmines.py View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/code-stubs.cc View 1 5 chunks +60 lines, -6 lines 0 comments Download
M src/globals.h View 1 1 chunk +10 lines, -4 lines 0 comments Download
M src/type-feedback-vector-inl.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/type-info.cc View 3 chunks +13 lines, -10 lines 0 comments Download

Messages

Total messages: 34 (20 generated)
mythria
Hi Benedikt and Leszek, I added support to collect feedback about Oddballs to Subtract stub. ...
4 years, 2 months ago (2016-10-10 11:32:16 UTC) #4
Benedikt Meurer
LGTM from my side.
4 years, 2 months ago (2016-10-10 11:37:16 UTC) #5
Leszek Swirski
https://codereview.chromium.org/2406843002/diff/1/src/code-stubs.cc File src/code-stubs.cc (right): https://codereview.chromium.org/2406843002/diff/1/src/code-stubs.cc#newcode840 src/code-stubs.cc:840: if_lhsisnotnumber(assembler), check_rhsisoddball(assembler), move the new label definitions to the ...
4 years, 2 months ago (2016-10-10 11:57:22 UTC) #8
mythria
Thanks Leszek and Benedikt. I addressed comments from Leszek. PTAL. Thanks, Mythri https://codereview.chromium.org/2406843002/diff/1/src/code-stubs.cc File src/code-stubs.cc ...
4 years, 2 months ago (2016-10-10 13:26:00 UTC) #9
Leszek Swirski
Great, LGTM from me as well.
4 years, 2 months ago (2016-10-10 13:28:04 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406843002/20001
4 years, 2 months ago (2016-10-10 14:19:34 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-10 14:22:16 UTC) #18
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/d5d283b27d406486e2f8d7b44c6d2b3db4f98458 Cr-Commit-Position: refs/heads/master@{#40124}
4 years, 2 months ago (2016-10-10 14:22:34 UTC) #20
mythria
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2407923002/ by mythria@chromium.org. ...
4 years, 2 months ago (2016-10-10 16:38:40 UTC) #21
Michael Achenbach
Is the windows error authentic? We had recently other weird windows errors that pointed to ...
4 years, 2 months ago (2016-10-11 06:58:39 UTC) #23
Michael Achenbach
Hmja, looks like we have proof for a build deps problem. The second tryjob ran ...
4 years, 2 months ago (2016-10-11 09:44:37 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406843002/40001
4 years, 2 months ago (2016-10-11 12:14:26 UTC) #30
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 2 months ago (2016-10-11 12:41:58 UTC) #32
commit-bot: I haz the power
4 years, 2 months ago (2016-10-11 12:42:12 UTC) #34
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/65716011492fce55475f6e57fc3e8d588afede88
Cr-Commit-Position: refs/heads/master@{#40170}

Powered by Google App Engine
This is Rietveld 408576698