Revision: 2531
Author: yu...@chromium.org
Date: Thu Jul 23 23:14:23 2009
Log: Debugger should not stop in its own code and in code of built-in
functions since it may confuse user.Debug break handler checks whether
current function is a built-in or a debugger one and just resumes execution
if
http://codereview.chromium.org/160041/diff/1001/1002
File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/160041/diff/1001/1002#newcode473
Line 473: __ ldr(reg, FieldMemOperand(ip, AccessorInfo::kDataOffset));
On 2009/07/24 00:30:07, iposva wrote:
> On 2009/07/23 21:12:58, anto
http://codereview.chromium.org/160041/diff/1001/1002
File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/160041/diff/1001/1002#newcode473
Line 473: __ ldr(reg, FieldMemOperand(ip, AccessorInfo::kDataOffset));
On 2009/07/23 21:12:58, antonm wrote:
> why not push data immediatel
overall LGTM.
And very neat trick to solve the problem of data in new space.
Some minor comments. You might want to compare it to
http://codereview.chromium.org/155682 which uses your cool trick as well
http://codereview.chromium.org/160041/diff/1001/1002
File src/arm/stub-cache-arm.cc (right
Reviewers: Christian Plesner Hansen, antonm,
Description:
Stub Cache: speed up load callback accessor by allocating data handle on
stack.
Please review this at http://codereview.chromium.org/160041
Affected files:
M src/arm/stub-cache-arm.cc
M src/ia32/stub-cache-ia32.cc
M src/stub-cac
http://codereview.chromium.org/159264/diff/1/3
File src/spaces.cc (left):
http://codereview.chromium.org/159264/diff/1/3#oldcode136
Line 136: default:
What was the complaint by Coverity here? It is really helpful to have
default cases marked as UNREACHABLE once you start adding new entries to
th
Lgtm!
http://codereview.chromium.org/160001
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Please also identify what happens on other platforms when compiled with
gcc.
-Ivan
http://codereview.chromium.org/13612/diff/4401/4403
File src/ia32/cpu-ia32.cc (right):
http://codereview.chromium.org/13612/diff/4401/4403#newcode31
Line 31: #include "third_party/valgrind/valgrind.h"
The code
btw, I believe the same annotation should be added to x64/cpu-x64.cc
On Thu, Jul 23, 2009 at 7:11 PM, Mads Sig Ager wrote:
> I'll run this through some more tests tomorrow and patch it in if it
> does not hurt.
>
> -- Mads
>
> On Thu, Jul 23, 2009 at 4:22 PM, Dean McNamee wrote:
>>
>> Mads, is th
I'll run this through some more tests tomorrow and patch it in if it
does not hurt.
-- Mads
On Thu, Jul 23, 2009 at 4:22 PM, Dean McNamee wrote:
>
> Mads, is this within the noise tolerance?
>
> Otherwise we can try caching whether or not valgrind is present.
>
> On Wed, Jul 22, 2009 at 12:35 PM
Revision: 2530
Author: a...@chromium.org
Date: Thu Jul 23 07:57:42 2009
Log: Push revisions 2523, 2527 and 2528 to 1.2 branch to fix bug where a
floating-point number could be interpreted as a string.
Review URL: http://codereview.chromium.org/159268
http://code.google.com/p/v8/source/detail?r=253
LGTM.
http://codereview.chromium.org/159268
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Reviewers: Kevin Millikin,
Description:
Push revisions 2523, 2527 and 2528 to 1.2 branch to fix bug where a
floating-point number could be interpreted as a string.
Please review this at http://codereview.chromium.org/159268
SVN Base: http://v8.googlecode.com/svn/branches/1.2/
Affected files:
Status: Assigned
Owner: kmilli...@chromium.org
Labels: Type-Bug Priority-Medium
New issue 407 by kmilli...@chromium.org: Snapshot deserialization may
overflow its own page tables
http://code.google.com/p/v8/issues/detail?id=407
The heap snapshot deserialization implementation uses a table of p
Mads, is this within the noise tolerance?
Otherwise we can try caching whether or not valgrind is present.
On Wed, Jul 22, 2009 at 12:35 PM, Timur Iskhodzhanov wrote:
> I've run
> $ cd benchmarks; ../shell run.js
> with the original (r2487) and annotated version of V8.
>
> Before each run I did
Revision: 2529
Author: ant...@chromium.org
Date: Thu Jul 23 06:36:28 2009
Log: Force inlining of some handle methods.
Review URL: http://codereview.chromium.org/159236
http://code.google.com/p/v8/source/detail?r=2529
Modified:
/branches/bleeding_edge/include/v8.h
=
Christian, please look at regexp-macro-assembler-ia32.cc changes.
http://codereview.chromium.org/160001
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM, gotta love that exd register. ;-)
http://codereview.chromium.org/160009
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Revision: 2526
Author: kmilli...@chromium.org
Date: Thu Jul 23 05:56:45 2009
Log: Call the (fatal) V8 out of memory handler if we cannot allocate enough
memory from the OS to deserialize the initial heap snapshot at
startup.
This catches the failure to startup earlier, and avoids dereferencing
th
Revision: 2528
Author: whe...@chromium.org
Date: Thu Jul 23 06:05:36 2009
Log: Fix typo in last change
Review URL: http://codereview.chromium.org/160009
http://code.google.com/p/v8/source/detail?r=2528
Modified:
/branches/bleeding_edge/src/ia32/ic-ia32.cc
==
Reviewers: Mads Ager,
Message:
tbr=a...@chromium.org
Description:
Fix typo in last change
Please review this at http://codereview.chromium.org/160009
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/ia32/ic-ia32.cc
Index: src/ia32/ic-ia32.cc
=
Revision: 2527
Author: whe...@chromium.org
Date: Thu Jul 23 06:01:17 2009
Log: Fix an error in a keyed lookup stub - HeapNumbers treated as strings.
Review URL: http://codereview.chromium.org/155924
http://code.google.com/p/v8/source/detail?r=2527
Added:
/branches/bleeding_edge/test/mjsunit/reg
Reviewers: iposva,
Message:
This fails a couple of tests. I am working on fixing it.
Description:
Add inline caching for keyed loads and stores. Remove extra parentheses
from some files.
Please review this at http://codereview.chromium.org/159266
SVN Base: http://v8.googlecode.com/svn/branch
LGTM, thanks!
http://codereview.chromium.org/155924
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/159265
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
http://codereview.chromium.org/155924/diff/2003/1016
File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/155924/diff/2003/1016#newcode294
Line 294: __ CmpInstanceType(edx, FIRST_SYMBOL_TYPE);
On 2009/07/23 11:41:51, Mads Ager wrote:
> I find this misleading since FIRST_SYMBOL_TYPE i
Revision: 2525
Author: kmilli...@chromium.org
Date: Thu Jul 23 05:51:49 2009
Log: Change a few style issues (dead code, unitialized members) flagged by
Coverity Prevent. All are benign.
Review URL: http://codereview.chromium.org/159264
http://code.google.com/p/v8/source/detail?r=2525
Modified:
LGTM
http://codereview.chromium.org/159264
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
Reviewers: Mads Ager,
Description:
Call the (fatal) V8 out of memory handler if we cannot allocate enough
memory from the OS to deserialize the initial heap snapshot at
startup.
This catches the failure to startup earlier, and avoids dereferencing
the encoding of an allocation failure.
Please
Reviewers: Mads Ager,
Description:
Change a few style issues (dead code, unitialized members) flagged by
Coverity Prevent. All are benign.
Please review this at http://codereview.chromium.org/159264
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/b
Updates:
Status: Fixed
Comment #1 on issue 406 by kmilli...@chromium.org: (ARM) Constant folding
crashes short-circuit boolean expressions
http://code.google.com/p/v8/issues/detail?id=406
Fixed in bleeding_edge r2524.
--
You received this message because you are listed in the owner
or
I would prefer to keep the old style check instead of forcing a use of
CmpInstanceType.
http://codereview.chromium.org/155924/diff/2003/1016
File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/155924/diff/2003/1016#newcode294
Line 294: __ CmpInstanceType(edx, FIRST_SYMBOL_TYPE);
I
Revision: 2524
Author: kmilli...@chromium.org
Date: Thu Jul 23 04:40:14 2009
Log: Fix ARM compiler crash in short-circuited boolean expressions.
We did not handle the case where the left-hand-side expression was
fully compiled to control flow. There were also some assertions for
unary and binary
LGTM
http://codereview.chromium.org/160006
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
It's not consistent with Chrome or Google coding style.
On Thu, Jul 23, 2009 at 12:20 PM, Christian Plesner
Hansen wrote:
> Having it on the declaration is consistent with our existing code.
> See for instance Arguments or ScriptOrigin.
>
> On Thu, Jul 23, 2009 at 12:15 PM, Anton Muhin wrote:
>>
Having it on the declaration is consistent with our existing code.
See for instance Arguments or ScriptOrigin.
On Thu, Jul 23, 2009 at 12:15 PM, Anton Muhin wrote:
> Yes, I saw it already. But I think standard preempts lite faq :)
>
> yours,
> anton.
>
> On Thu, Jul 23, 2009 at 2:13 PM, Dean McN
Yes, I saw it already. But I think standard preempts lite faq :)
yours,
anton.
On Thu, Jul 23, 2009 at 2:13 PM, Dean McNamee wrote:
> http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.9
>
> On Thu, Jul 23, 2009 at 12:10 PM, Anton Muhin wrote:
>> Dean, are you sure?
>>
>> Holy st
http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.9
On Thu, Jul 23, 2009 at 12:10 PM, Anton Muhin wrote:
> Dean, are you sure?
>
> Holy standard says:
>
> A function declaration (8.3.5, 9.3, 11.4) with an inline specifier
> declares an inline function. The inline
> specifier indic
Dean, are you sure?
Holy standard says:
A function declaration (8.3.5, 9.3, 11.4) with an inline specifier
declares an inline function. The inline
specifier indicates to the implementation that inline substitution of
the function body at the point of call is to be preferred to the usual
function
I am not sure this is right, the inline keyword goes on the
definition, not the declaration.
On Thu, Jul 23, 2009 at 9:35 AM, wrote:
>
> Oh, Christian just pointed out to me that these methods are template
> methods and therefore the implementation is in the header file. :-)
>
> LGTM
>
>
>
> htt
Reviewers: Mads Ager,
Description:
Fix ARM compiler crash in short-circuited boolean expressions.
We did not handle the case where the left-hand-side expression was
fully compiled to control flow. There were also some assertions for
unary and binary expressions that crashed debug builds when th
Updates:
Status: Fixed
Comment #2 on issue 405 by whe...@chromium.org: Evaluate performance impact
of calling GenerateDictionaryLoad in LoadKeyedIC::GenerateGeneric
http://code.google.com/p/v8/issues/detail?id=405
(No comment was entered for this change.)
--
You received this message
http://codereview.chromium.org/155924/diff/3/1008
File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/155924/diff/3/1008#newcode51
Line 51: // complete runtime version.
On 2009/07/22 18:59:55, iposva wrote:
> Bill since we only ever enter symbols into the dictionaries as keys
execut
Status: Accepted
Owner: kmilli...@chromium.org
CC: f...@chromium.org
Labels: Type-Bug Priority-High HW-ARM
New issue 406 by kmilli...@chromium.org: (ARM) Constant folding crashes
short-circuit boolean expressions
http://code.google.com/p/v8/issues/detail?id=406
In the ARM code generator, const
Oh, Christian just pointed out to me that these methods are template
methods and therefore the implementation is in the header file. :-)
LGTM
http://codereview.chromium.org/159236
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.g
Actually, does this make a difference at all. The implementation is not
in a header file, so there seems to be no way for the compiler to inline
these anyway?
http://codereview.chromium.org/159236
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
LGTM as well.
http://codereview.chromium.org/159236
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
LGTM
http://codereview.chromium.org/160001
--~--~-~--~~~---~--~~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~--~~~~--~~--~--~---
On Thu, Jul 23, 2009 at 10:09 AM, wrote:
> Lgtm.
>
> How much of a different does it make in performance?
Pretty minor if any. But we'd better not have them visible at all IMHO.
yours,
anton.
>
> http://codereview.chromium.org/159236
>
--~--~-~--~~~---~--~~
v8
49 matches
Mail list logo
