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

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:
4 years, 2 months ago by Alexander Potapenko
Modified:
4 years ago
Reviewers:
danno, Jakob Kummerow
CC:
v8-dev, 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) Patch
M src/atomicops.h View 1 1 chunk +3 lines, -1 line 0 comments Download
A src/atomicops_internals_tsan.h View 1 2 3 4 5 6 1 chunk +335 lines, -0 lines 1 comment Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 10 (0 generated)
Alexander Potapenko
Hi Daniel, can you please take a look? This change is required for TSan v2 ...
4 years, 1 month ago (2012-11-09 14:17:12 UTC) #1
Alexander Potapenko
CC dvyukov@
4 years, 1 month ago (2012-11-09 14:36:13 UTC) #2
Alexander Potapenko
+jkummerow Jakob, any chance you can review this?
4 years ago (2012-11-12 14:04:26 UTC) #3
danno
The license and documentation seem to be taken directly from Chromium (e.g. copyright and base/atomicops.h ...
4 years ago (2012-11-12 14:51:45 UTC) #4
Alexander Potapenko
Fixed the license headers and most of the lint errors. The remaining lint errors in ...
4 years ago (2012-11-12 16:50:24 UTC) #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 ...
4 years ago (2012-11-13 15:43:22 UTC) #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: ...
4 years ago (2012-11-13 16:45:00 UTC) #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): ...
4 years ago (2012-11-13 21:28:58 UTC) #8
Alexander Potapenko
Is it fine to bump the V8 revision at Chrome, or you're only using stable ...
4 years ago (2012-11-14 10:08:43 UTC) #9
danno
4 years ago (2012-11-14 10:11:06 UTC) #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 f8e48bd