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

Issue 667923004: Go generic on polymorphic KeyedLoads with string+other maps (Closed)

Created:
6 years, 2 months ago by mvstanton
Modified:
6 years, 2 months ago
Reviewers:
Jarin
CC:
v8-dev
Project:
v8
Visibility:
Public.

Description

The issue is that by handling strings with map/handler pairs instead of a special version of the keyed load stub (https://code.google.com/p/v8/source/detail?r=24661), I allowed polymorphism between string and non-string types in the IC. Before, the IC would go generic. Then, at crankshaft time, we special case when we only saw strings. The error here is that crankshaft can't emit code that handles polymorphism between string and non-string types. The choice is either to get that to happen (I don't deem this necessary from a performance point of view, an IC with such type feedback before would have gone generic), or simply check for the case of "polymorphic with some string maps" and require crankshaft to go generic. I'll do the latter. BUG=425519 LOG=N R=jarin@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=24775

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -6 lines) Patch
M src/hydrogen.cc View 2 chunks +14 lines, -0 lines 0 comments Download
A + test/mjsunit/regress/regress-crbug-425519.js View 1 chunk +7 lines, -6 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
mvstanton
Hi Jaroslav, could you have a look? Thanks! --Michael
6 years, 2 months ago (2014-10-21 12:42:43 UTC) #2
Jarin
lgtm. Thanks!
6 years, 2 months ago (2014-10-21 12:47:47 UTC) #3
mvstanton
6 years, 2 months ago (2014-10-21 13:05:01 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as 24775 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698