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

Issue 2716253003: Revert of [SAB] Move Atomics builtins to C++ (Closed)

Created:
3 years, 9 months ago by Michael Hablich
Modified:
3 years, 9 months ago
Reviewers:
Dan Ehrenberg, binji
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Revert of [SAB] Move Atomics builtins to C++ (patchset #10 id:170001 of https://codereview.chromium.org/2698813004/ ) Reason for revert: Seems to the culprit in BUG=chromium:695653 Original issue's description: > [SAB] Move Atomics builtins to C++ > > This patch refactors the Atomics builtins so that they are implemented > as C++ builtins rather than experimental JS builtins. Previously, each > of these functions called out to a runtime function, so no significant > change in performance is anticipated. The goal of this patch is to > remove the last user of experimental JS builtins so that the mechanism > can be removed, for performance reasons. The patch includes a drive-by > fix of a check-fail. For the most part, the patch is just moving code > without modification from runtime-atomics.cc to > builtins-sharedarraybuffer.cc . > > BUG=v8:5880 > > Review-Url: https://codereview.chromium.org/2698813004 > Cr-Commit-Position: refs/heads/master@{#43335} > Committed: https://chromium.googlesource.com/v8/v8/+/2b9840d86f3b51f42918b6bf1a06ff8b62b2464d TBR=binji@chromium.org,littledan@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=v8:5880

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -7 lines) Patch
M src/builtins/builtins-sharedarraybuffer.cc View 8 chunks +63 lines, -7 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
Michael Hablich
Created Revert of [SAB] Move Atomics builtins to C++
3 years, 9 months ago (2017-02-27 18:54:47 UTC) #2
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/2716253003/1
3 years, 9 months ago (2017-02-27 18:54:59 UTC) #3
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/builds/13805) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, ...
3 years, 9 months ago (2017-02-27 18:56:09 UTC) #5
Michael Hablich
3 years, 9 months ago (2017-02-27 19:09:07 UTC) #6
meh, of course it does not apply cleanly anymore.

Powered by Google App Engine
This is Rietveld 408576698