Please have another look. As discussed offline I changed the logic that
puts the
error object into the code place to instead write a specific smi
(kCompilationError) and put the error object at the saved index instead. In
addition, I put the mask into the RegExp class and assert that we always
have a
valid smi in the RegExp compiler.
Also, unrelated, but removed some trailing whitespaces in two js files.
On 2011/07/01 05:58:09, Rico wrote:
All comments addressed, will check with Slava if we can skip the write
barrier
entirely.
http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/arm/code-stubs-arm.cc#newcode4425
src/arm/code-stubs-arm.cc:4425: __ b(ne, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> This b is superfluous
Done.
http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/ia32/code-stubs-ia32.cc#newcode3385
src/ia32/code-stubs-ia32.cc:3385: __ j(not_equal, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> This j is superfluous.
Done.
http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc
File src/jsregexp.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/jsregexp.cc#newcode298
src/jsregexp.cc:298: // We could potentially have marked this as
flushable,
but
have keept
On 2011/06/30 18:48:25, Erik Corry wrote:
> keept -> kept
Done.
http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc
File src/mark-compact.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/mark-compact.cc#newcode733
src/mark-compact.cc:733: if (value == (heap->sweep_generation() -
kRegExpCodeThreshold) % 256) {
On 2011/06/30 18:48:25, Erik Corry wrote:
> If the lhs is negative then according to the standard the result of
the %
> operator can be either positive or negative. Therefore I would prefer
using
&
> 0xff which can yield only a positive answer. The same applies above.
Done.
http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc
File src/objects-debug.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode472
src/objects-debug.cc:472: ASSERT(ascii_data->IsSmi() ||
ascii_data->IsJSObject()
||
On 2011/06/30 18:48:25, Erik Corry wrote:
> I can see that it was already there before, but it is not clear to me
how
this
> can be a JSObject?
Compilation error, see comment two lines above?
http://codereview.chromium.org/7282026/diff/7018/src/objects-debug.cc#newcode478
src/objects-debug.cc:478: Object* ascii_saved =
arr->get(JSRegExp::kIrregexpUC16CodeSavedIndex);
On 2011/06/30 18:48:25, Erik Corry wrote:
> Should be the ASCII index here.
Ups, nice spot
http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h
File src/objects-inl.h (right):
http://codereview.chromium.org/7282026/diff/7018/src/objects-inl.h#newcode3914
src/objects-inl.h:3914: fa->set_unchecked(heap, index, value,
UPDATE_WRITE_BARRIER);
On 2011/06/30 18:48:25, Erik Corry wrote:
> I think this is OK, but I would like to ask Slava.
Done.
http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h
File src/objects-visiting.h (right):
http://codereview.chromium.org/7282026/diff/7018/src/objects-visiting.h#newcode304
src/objects-visiting.h:304: table_.Register(kVisitJSRegExp,
On 2011/06/30 18:48:25, Erik Corry wrote:
> Fits on one line.
Done.
http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):
http://codereview.chromium.org/7282026/diff/7018/src/x64/code-stubs-x64.cc#newcode2460
src/x64/code-stubs-x64.cc:2460: __ j(not_equal, &runtime);
On 2011/06/30 18:48:25, Erik Corry wrote:
> Hat trick!
Done.
http://codereview.chromium.org/7282026/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
