lgtm with some nits and non-blocking questions.
https://codereview.chromium.org/1162503002/diff/60001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/1162503002/diff/60001/src/objects.h#newcode6913
src/objects.h:6913: ATOMIC_FUNCTIONS_WITH_ID_LIST(DECLARE_FUNCTION_ID)
Funny indentation.
https://codereview.chromium.org/1162503002/diff/60001/src/runtime/runtime-atomics.cc
File src/runtime/runtime-atomics.cc (right):
https://codereview.chromium.org/1162503002/diff/60001/src/runtime/runtime-atomics.cc#newcode21
src/runtime/runtime-atomics.cc:21: #define REQUIRE_LOCK_64_BIT 0
This should go into some header file because it has to match the
compiler (Turbofan) versions of the atomics.
In fact, the __atomic* stuff below is a bit scary because we are relying
on the C++ libraries doing the same thing as the Turbofan version.
Ultimately, we might want to create a stub or some such for the atomics
to ensure consistency between the optimized and unoptimized versions.
Anyway, this does not have to be addressed by this CL.
https://codereview.chromium.org/1162503002/diff/60001/src/runtime/runtime-atomics.cc#newcode23
src/runtime/runtime-atomics.cc:23: bool IsLockFree(uint32_t size) {
Channeling bmeurer@: The file-local methods should go into anonymous
namespace(s).
https://codereview.chromium.org/1162503002/diff/60001/src/runtime/runtime-atomics.cc#newcode39
src/runtime/runtime-atomics.cc:39: #if V8_CC_GNU
This platform-specific bifurcation is a bit ugly, perhaps we should use
atomicops.h after all (with the compare-and-swap loops for
read-modify-writes and extra barriering to get sequential consistency).
The runtime functions will be slow anyway, so the extra cost should not
matter. What do you think?
https://codereview.chromium.org/1162503002/diff/60001/test/mjsunit/harmony/atomics.js
File test/mjsunit/harmony/atomics.js (right):
https://codereview.chromium.org/1162503002/diff/60001/test/mjsunit/harmony/atomics.js#newcode14
test/mjsunit/harmony/atomics.js:14: // {constr: Uint8ClampedArray, min:
0, max: 255},
The spec has the clamped arrays, so I guess we should support them.
https://codereview.chromium.org/1162503002/
--
--
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/d/optout.
