I know that this is massively annoying, but AtomicWord that could
participate in
a data race should never be cast to a normal pointer (undefined behavior in
C++11). Instead, you should always use the Store/Load function to access the
atomic memory. Sadly, this might require some pretty awkward code - I
suggested
one solution in the comments, but you might come up with something prettier.
Thanks for doing this, it is great that you are cleaning up our parallelism
story.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc
File src/spaces.cc (right):
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2085
src/spaces.cc:2085: if (NoBarrier_Load(&category->top_) != 0) {
Use the top() accessor.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2098
src/spaces.cc:2098: NoBarrier_Store(&top_, category->top_);
Use the set_top() and top() accessors.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2115
src/spaces.cc:2115: FreeListNode** n = GetTopAddress();
Replace with
FreeListNode* t = top();
FreeListNode** n = &t;
.... loop ....
set_top(t);
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2134
src/spaces.cc:2134: FreeListNode** n = GetTopAddress();
Replace with the gymnastics with accessors (or unroll the loop once).
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h
File src/spaces.h (right):
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1525
src/spaces.h:1525: return reinterpret_cast<FreeListNode**>(&top_);
Unfortunately, this would not be legal by the C++11 spec - once a
reference is AtomicWord, it should only be accessed via the
(Release|Acquire|NoBarrier)_(Load|Store) functions. It should never be
cast to a normal pointer. We will have to get rid of this method and use
the normal accessors before and after the loop where the method is used
(see the comments in spaces.cc).
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1529
src/spaces.h:1529: return reinterpret_cast<FreeListNode*>(top_);
Please, use NoBarrier_Load here.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1533
src/spaces.h:1533: top_ = reinterpret_cast<AtomicWord>(top);
NoBarrier_Store.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1547
src/spaces.h:1547: return top_ == 0;
Use the accessor rather than the direct access.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1556
src/spaces.h:1556: AtomicWord top_;
At the end of the line, add // FreeListNode*
https://codereview.chromium.org/138953018/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
