Chromium Code Reviews
Help | Chromium Project | Sign in
(678)

Issue 10965035: Add atomics implementation for ThreadSanitizer v2 (https://sites.google.com/a/chromium.org/dev/deve… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by Alexander Potapenko
Modified:
1 year, 5 months ago
Reviewers:
danno, Jakob
CC:
v8-dev_googlegroups.com, dvyukov
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 2

Patch Set 7 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+338 lines, -1 line) Lint Patch
M src/atomicops.h View 1 1 chunk +3 lines, -1 line 0 comments 2 errors Download
A src/atomicops_internals_tsan.h View 1 2 3 4 5 6 1 chunk +335 lines, -0 lines 1 comment 3 errors Download
Trybot results:
Commit:

Messages

Total messages: 10
Alexander Potapenko
Hi Daniel, can you please take a look? This change is required for TSan v2 ...
1 year, 5 months ago #1
Alexander Potapenko
CC dvyukov@
1 year, 5 months ago #2
Alexander Potapenko
+jkummerow Jakob, any chance you can review this?
1 year, 5 months ago #3
danno
The license and documentation seem to be taken directly from Chromium (e.g. copyright and base/atomicops.h ...
1 year, 5 months ago #4
Alexander Potapenko
Fixed the license headers and most of the lint errors. The remaining lint errors in ...
1 year, 5 months ago #5
danno
Almost there... https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_tsan.h File src/atomicops_internals_tsan.h (right): https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_tsan.h#newcode164 src/atomicops_internals_tsan.h:164: Atomic32 old_value, nit: parameter indentation? Here and ...
1 year, 5 months ago #6
Alexander Potapenko
https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_tsan.h File src/atomicops_internals_tsan.h (right): https://codereview.chromium.org/10965035/diff/7002/src/atomicops_internals_tsan.h#newcode164 src/atomicops_internals_tsan.h:164: Atomic32 old_value, On 2012/11/13 15:43:22, danno wrote: > nit: ...
1 year, 5 months ago #7
danno
lgtm with nit, I'll fix it and land it for you. http://codereview.chromium.org/10965035/diff/12003/src/atomicops_internals_tsan.h File src/atomicops_internals_tsan.h (right): ...
1 year, 5 months ago #8
Alexander Potapenko
Is it fine to bump the V8 revision at Chrome, or you're only using stable ...
1 year, 5 months ago #9
danno
1 year, 5 months ago #10
On 2012/11/14 10:08:43, Alexander Potapenko wrote:
> Is it fine to bump the V8 revision at Chrome, or you're only using stable
> revisions?

No, it's not okay to bump the version. You will have to wait until our next V8
push to trunk to complete (probably today).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434