On 2014/02/13 12:15:51, Hannes Payer wrote:
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) {
On 2014/02/12 21:38:02, jarin wrote:
> Use the top() accessor.
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2098
src/spaces.cc:2098: NoBarrier_Store(&top_, category->top_);
On 2014/02/12 21:38:02, jarin wrote:
> Use the set_top() and top() accessors.
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2115
src/spaces.cc:2115: FreeListNode** n = GetTopAddress();
On 2014/02/12 21:38:02, jarin wrote:
> Replace with
>
> FreeListNode* t = top();
> FreeListNode** n = &t;
>
> .... loop ....
>
> set_top(t);
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.cc#newcode2134
src/spaces.cc:2134: FreeListNode** n = GetTopAddress();
On 2014/02/12 21:38:02, jarin wrote:
> Replace with the gymnastics with accessors (or unroll the loop once).
Done.
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_);
On 2014/02/12 21:38:02, jarin wrote:
> 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).
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1529
src/spaces.h:1529: return reinterpret_cast<FreeListNode*>(top_);
On 2014/02/12 21:38:02, jarin wrote:
> Please, use NoBarrier_Load here.
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1533
src/spaces.h:1533: top_ = reinterpret_cast<AtomicWord>(top);
On 2014/02/12 21:38:02, jarin wrote:
> NoBarrier_Store.
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1547
src/spaces.h:1547: return top_ == 0;
On 2014/02/12 21:38:02, jarin wrote:
> Use the accessor rather than the direct access.
Done.
https://codereview.chromium.org/138953018/diff/50001/src/spaces.h#newcode1556
src/spaces.h:1556: AtomicWord top_;
On 2014/02/12 21:38:02, jarin wrote:
> At the end of the line, add // FreeListNode*
Done.
lgtm with a couple of questions.
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.
