Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(307)

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

Created:
5 years, 2 months ago by Alexander Potapenko
Modified:
5 years, 1 month 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:

Messages

Total messages: 10 (0 generated)
Alexander Potapenko
Hi Daniel, can you please take a look? This change is required for TSan v2 ...
5 years, 1 month ago (2012-11-09 14:17:12 UTC) #1
Alexander Potapenko
CC dvyukov@
5 years, 1 month ago (2012-11-09 14:36:13 UTC) #2
Alexander Potapenko
+jkummerow Jakob, any chance you can review this?
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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 ...
5 years, 1 month 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: ...
5 years, 1 month 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): ...
5 years, 1 month 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 ...
5 years, 1 month ago (2012-11-14 10:08:43 UTC) #9
danno
5 years, 1 month 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).

Powered by Google App Engine
This is Rietveld 0eb02b776