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

Issue 2295743002: [wasm] Refactor BuildWasmToJSWrapper to clearly separate direct calls from Call-codestub calls (Closed)

Created:
4 years, 3 months ago by ahaas
Modified:
4 years, 3 months ago
Reviewers:
titzer
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[wasm] Refactor BuildWasmToJSWrapper to clearly separate direct calls from Call-codestub calls Looking at the code with Toon showed me that the code is not really readable at the moment. This refactoring should make the different kinds of calls and their parameters more apparent. R=titzer@chromium.org Committed: https://crrev.com/e809937eb33e4cfafd062f0f2baa89e544ba9c49 Cr-Commit-Position: refs/heads/master@{#39126}

Patch Set 1 #

Total comments: 2

Patch Set 2 : factor out loop #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -32 lines) Patch
M src/compiler/wasm-compiler.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M src/compiler/wasm-compiler.cc View 1 4 chunks +45 lines, -32 lines 0 comments Download

Messages

Total messages: 17 (10 generated)
ahaas
4 years, 3 months ago (2016-08-30 12:08:08 UTC) #1
titzer
https://codereview.chromium.org/2295743002/diff/1/src/compiler/wasm-compiler.cc File src/compiler/wasm-compiler.cc (right): https://codereview.chromium.org/2295743002/diff/1/src/compiler/wasm-compiler.cc#newcode2573 src/compiler/wasm-compiler.cc:2573: // Convert WASM numbers to JS values. Is it ...
4 years, 3 months ago (2016-09-01 14:35:17 UTC) #6
ahaas
https://codereview.chromium.org/2295743002/diff/1/src/compiler/wasm-compiler.cc File src/compiler/wasm-compiler.cc (right): https://codereview.chromium.org/2295743002/diff/1/src/compiler/wasm-compiler.cc#newcode2573 src/compiler/wasm-compiler.cc:2573: // Convert WASM numbers to JS values. On 2016/09/01 ...
4 years, 3 months ago (2016-09-01 17:17:52 UTC) #9
titzer
lgtm
4 years, 3 months ago (2016-09-02 07:39:50 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/2295743002/20001
4 years, 3 months ago (2016-09-02 10:52:52 UTC) #14
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 3 months ago (2016-09-02 10:54:50 UTC) #15
commit-bot: I haz the power
4 years, 3 months ago (2016-09-02 10:55:40 UTC) #17
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/e809937eb33e4cfafd062f0f2baa89e544ba9c49
Cr-Commit-Position: refs/heads/master@{#39126}

Powered by Google App Engine
This is Rietveld 408576698