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

Issue 689403002: [turbofan] Do not use the generic graph algorithm for widening in the typer. (Closed)

Created:
6 years, 1 month ago by Jarin
Modified:
6 years, 1 month ago
Reviewers:
Benedikt Meurer
CC:
rossberg, v8-dev
Project:
v8
Visibility:
Public.

Description

[turbofan] Do not use the generic graph algorithm for widening in the typer. This change uses an explicit queue for type-widening instead of the generic algorithm. The trouble with the generic algorithm was that it called the visitor on the same phi many times in a row (and thus caused unnecessary retyping). I also think that the queue-based fixpoint is more readable. The CL cuts running time of the nbody-java benchmark from ~19s to ~15s, the time spent in the typer goes from 4.5s to 1s. This is still a lot - the root cause appears to be slow handling of union subtyping (m*n for unions of sizes m and n). I see a re-typing of a single phi node taking > 100ms. I will work on a fix with Andreas, hopefully we can come up with some canonical representation of unions at least for the common cases (union of Smi constants). I have also changed the initial typer run to always compute a type, even if we already had a type for the node. This fixes one assert failure where context specialization updates a node without updating the type, which confuses the typer when widening (as some types suddenly narrow). BUG= R=bmeurer@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=25053

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -23 lines) Patch
M src/compiler/typer.cc View 3 chunks +52 lines, -23 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
Jarin
Could you take a look, please?
6 years, 1 month ago (2014-11-02 12:06:35 UTC) #2
Benedikt Meurer
LGTM
6 years, 1 month ago (2014-11-03 05:36:30 UTC) #3
Jarin
6 years, 1 month ago (2014-11-03 06:09:59 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as 25053 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698