On Thu, Sep 11, 2008 at 3:55 PM, <[EMAIL PROTECTED]> wrote:
> LGTM. I still think you should consider renaming the .defs file to
> something else.
>
>
> http://codereview.chromium.org/1935/diff/53/270
> File src/flags.cc (right):
>
> http://codereview.chromium.org/1935/diff/53/270#newcode50
> Lin
On Thu, Sep 11, 2008 at 4:11 PM, <[EMAIL PROTECTED]> wrote:
> Hi Dean
>
> Great first step to move all flags to one file.
> However, it would be perfect if you could eliminate the flags.defs.
>
> Otherwise, LGTM,
> Lars
>
>
> http://codereview.chromium.org/1935/diff/53/271
> File src/flags.defs
suggested can be added later.
> LGTM
>
> Regards,
> Lars
>
> On Thu, Sep 11, 2008 at 4:15 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
>> On Thu, Sep 11, 2008 at 4:11 PM, <[EMAIL PROTECTED]> wrote:
>>> Hi Dean
>>>
>>> Great first step
LGTM, thanks
On Wed, Sep 17, 2008 at 12:27 PM, <[EMAIL PROTECTED]> wrote:
> Reviewers: deanm,
>
> Description:
> Move the initial code space to be before the young space instead of
> after it.
> The old arrangement meant that the alignment requirements of young space
> were
> likely to nullify A
Hmmm, I forgot about making the native method robust. How important
is this? I guess it's just good for quality over all. If you call
%StringIndexOf("a", "", 0) now, it will be bad, since both the KMP and
the brute force code expect a non-zero pattern. I will try to think
of a way to handle th
savings (~0.7%). When using a short jmp encoding (what this change
makes possible), it results in 88667 bytes, which is maybe a bit
better (~1.3%).
On Wed, Sep 24, 2008 at 3:46 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> So this reversing the forward branch from being likely not taken to
So this reversing the forward branch from being likely not taken to
likely taken, so that might throw off static branch prediction? Will
be interesting to see if there are any performance changes on
benchmarks.
On Wed, Sep 24, 2008 at 3:04 PM, <[EMAIL PROTECTED]> wrote:
> Reviewers: Kasper Lund
It is hard to tell with all of the p4 noise, but it seems to have
gotten some generally very strong improvements. Showing 14% on
fibonacci, although I'm skeptical. I guess will have to watch for a
few more checkins and see if it stays that way.
On Wed, Sep 24, 2008 at 6:27 PM, Kasper Lund <[EMA
There is nothing beyond O3, so O9 is kinda a bad joke. Also, any
guesses why GCC thinks those variables might go uninitialized? (I saw
the same problem).
Seconds, I do not think we should -mtune=nocona for a few reasons.
nocona is a NetBurst chip, so we would be tuning towards p4. If
anyone we
How about a regression test? I realize we found it via a regression
test already, but it was for another bug. Seems like maybe it would
be good to have a test for incorrect custom sort comparators?
On Fri, Sep 26, 2008 at 12:08 PM, Christian Plesner Hansen
<[EMAIL PROTECTED]> wrote:
>
> Tak så
Btw, this is a decent summary:
http://gentoo-wiki.com/CFLAGS
I think you may have the order wrong now? It seems like -march
implies -mtune, so does the -mtune need to be after the -march? Not
sure...
On Fri, Sep 26, 2008 at 4:56 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> There i
On Sat, Sep 27, 2008 at 12:59 AM, Erik Corry <[EMAIL PROTECTED]> wrote:
> I made the O9 to O3 and added volatile to the pointers intended to force fp
> values into memory.
>
> On Fri, Sep 26, 2008 at 6:37 PM, <[EMAIL PROTECTED]> wrote:
>>
>> The union trick is not defined in the standard, but appa
On Sat, Sep 27, 2008 at 1:15 AM, <[EMAIL PROTECTED]> wrote:
>
> Author: [EMAIL PROTECTED]
> Date: Fri Sep 26 16:14:43 2008
> New Revision: 389
>
> Modified:
>branches/bleeding_edge/src/checks.h
>
> Log:
> Fix failing test caused by extra precision in register allocated fp values.
> Review URL
I don't think I'm following. Bit cast should be solving the same
problem without the need for a union?
On Sat, Sep 27, 2008 at 1:29 AM, Erik Corry <[EMAIL PROTECTED]> wrote:
>
>
> On Sat, Sep 27, 2008 at 1:23 AM, Dean McNamee <[EMAIL PROTECTED]> wrote:
>>
I will try to look into ARM, and if it's not an advantage there, and
ok with Ivan, I will remove ARM jump elimination also.
On Mon, Sep 29, 2008 at 6:31 AM, Kasper Lund <[EMAIL PROTECTED]> wrote:
> Given the complexity of the code and the very small benefit it gives
> us, this change certainly LG
6, 2008 at 6:08 PM, Erik Corry <[EMAIL PROTECTED]> wrote:
>
>
> On Fri, Sep 26, 2008 at 4:56 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
>>
>> There is nothing beyond O3, so O9 is kinda a bad joke. Also, any
>> guesses why GCC thinks those variab
I am seeing no code-size difference on ARM with and without the jump
eliminator. I'm going to remove it, and replace it with an option to
print unneeded jumps that we see while doing fixups.
Thanks
On Mon, Sep 29, 2008 at 10:08 AM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> I wi
I am not sure this code is correct. On ARM, the push / pop eliminator
looks and make sure that there wasn't a label bound in between the
instructions. For example:
Label some_label;
j(&some_label);
push 4
bind(&some_label);
pop eax
The x86 jump eliminator doesn't seem to recognize this case, a
ush/pop pair (for literals, initially) in the first place. We should
> probably fix this now, though.
>
> On Mon, Sep 29, 2008 at 1:55 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
>>
>> I am not sure this code is correct. On ARM, the push / pop eliminator
>> l
I figured it was better to not dirty up the public header with a
macro, but I suppose I could undef it at the end. Not a huge deal
either way?
On Mon, Sep 29, 2008 at 8:17 PM, Christian Plesner Hansen
<[EMAIL PROTECTED]> wrote:
> Lgtm. You could consider using the standard macro instead of writ
about 20% reduction in the size of shell.exe but it is also
> about 10% slower on the V8 benchmark. Adding /Oi doesn't make much
> difference. Using /O2 and /Ox together with /GL (hole program optimization)
> gives the exact same size of shell.exe. /O2 it is.
> /Søren
>
> On Th
Btw, I was curious, so I just looked at the difference between /O1
(size) and /O2 (speed) and /Ox (full)
/O1 (Minimize Size) /Og /Os /Oy /Ob2 /Gs /GF /Gy
/O2 (Maximize Speed) /Og /Oi /Ot /Oy /Ob2 /Gs /GF /Gy
So the difference is the addition of /Oi, and /Ot. The /Oi means
things like, you can
Ok, dropping the review, too much work.
On Thu, Oct 2, 2008 at 12:21 PM, <[EMAIL PROTECTED]> wrote:
> Wouldn't it be more straightforward if we controlled this by a -j K
> option and divided the workload into chunks of #files / K?
>
> Also, we shouldn't parallelize by default if it messes up the
V8 is still about 40% of Chromium's static initializers. It seems
that static initializers are inversely proportional to number of lines
of code :)
v8___dynamic_initializer_for__thread_local__
v8___dynamic_initializer_for__write_input_buffer__
v8__internal___dynamic_initializer_for__string_input
TryFlatten ignores memory allocation failures. What happens when the
string can't be flattened?
On Mon, Oct 6, 2008 at 2:47 PM, <[EMAIL PROTECTED]> wrote:
>
> ...and here are my comments.
>
>
> http://codereview.chromium.org/6269/diff/1/5
> File src/runtime.cc (right):
>
> http://codereview.chr
Won't memcpy always be an inlined intrinsic?
On Wed, Oct 8, 2008 at 10:38 AM, Kevin Millikin <[EMAIL PROTECTED]> wrote:
> LGTM.
>
> On Wed, Oct 8, 2008 at 10:31 AM, <[EMAIL PROTECTED]> wrote:
>>
>> Reviewers: Kevin Millikin,
>>
>> Description:
>> Minor adjustments to the object migration code: Wh
ch to pick one.
On Wed, Oct 8, 2008 at 12:58 PM, Erik Corry <[EMAIL PROTECTED]> wrote:
>
>
> On Wed, Oct 8, 2008 at 12:10 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
>>
>> Won't memcpy always be an inlined intrinsic?
>
> Not with variable size:
>
>
Blah, it is in the header, sorry.
On Wed, Oct 22, 2008 at 11:45 AM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> Will CopyBlock always be inlined? so does it really have the same
> effect? Aren't you going to now have twice as much overhead, one for
> the call to CopyBlock,
Will CopyBlock always be inlined? so does it really have the same
effect? Aren't you going to now have twice as much overhead, one for
the call to CopyBlock, and another for the call to memcpy? Maybe you
should define it in the header if you really want it inlined?
On Wed, Oct 22, 2008 at 10:17
On the V8 benchmark:
bytes emitted (--log-code):
before 451007
after 446262
So about a 1% code size improvement.
-- dean
On Sun, Nov 9, 2008 at 10:18 AM, <[EMAIL PROTECTED]> wrote:
> Reviewers: Kasper Lund,
>
> Message:
> There is a bit of duplication for the mov(reg, reg), since it coul
I talked to Kasper, and I think the conclusion was to use Immediate(),
but I'm not positive. I'll see what he thinks. Thanks for the
review.
-- dean
On Mon, Nov 10, 2008 at 7:05 AM, <[EMAIL PROTECTED]> wrote:
> I like it. Even if it doesn't result in a shorter encoding, I think it
> reads be
patch makes sense.
Kasper?
On Mon, Nov 10, 2008 at 7:13 AM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> I talked to Kasper, and I think the conclusion was to use Immediate(),
> but I'm not positive. I'll see what he thinks. Thanks for the
> review.
>
> -- dean
>
It complains because it's actually a bug (order of precedence).
LGTM.
On Tue, Nov 11, 2008 at 10:37 AM, <[EMAIL PROTECTED]> wrote:
>
> Reviewers: iposva,
>
> Message:
> a small fix
>
> Description:
> cl.exe complains about missing parens around the boolean expression.
>
>
> Please review this a
Just an fyi, beware of:
http://securityvulns.com/advisories/year3000.asp
-- dean
On Mon, Dec 1, 2008 at 5:50 AM, <[EMAIL PROTECTED]> wrote:
>
> Reviewers: Mike Belshe, Mads Ager,
>
> Message:
> Here is the change in the daylight saving time lookup
> code. I also have a browser test for it, wh
The valgrind header is explicitly licensed for copying. From the manual:
"""
You are encouraged to copy the valgrind/*.h headers into your
project's include directory, so your program doesn't have a
compile-time dependency on Valgrind being installed. The Valgrind
headers, unlike most of the res
nt is that, in the future, we might want to expose a
> collection-like interface (or V8 understand a collection-like interface)
> so the API can use the collection interface rather than an array of
> pointers.
>
> On 2008/12/10 14:23:42, Dean McNamee wrote:
>>
>> I tried
I think that benchmarker is doubly bad because of moma's cache
headers. I will try to investigate something better tomorrow, but I
still think the win should be relatively the same.
-- dean
On Wed, Dec 10, 2008 at 5:36 PM, Dean McNamee <[EMAIL PROTECTED]> wrote:
> I just came
I hopefully addressed both of these comments. New snapshot uploaded.
On Wed, Dec 10, 2008 at 3:38 PM, <[EMAIL PROTECTED]> wrote:
> My $.02. Lgtm overall.
>
>
> http://codereview.chromium.org/13341/diff/1/2
> File include/v8.h (right):
>
> http://codereview.chromium.org/13341/diff/1/2#newcode19
I think the "correct" fix is to change webkit upstream. I also looked
at some of these message mismatches. The inconsistency is that webkit
uses all combinations of:
this is an error message
this is an error message.
This is an error message
This is an error message.
V8 is aiming to consistent
V8 is 32-bit only in a lot more ways that using intptr_t instead of
uint32. Infact, they are using uint32 correctly, because the code is
only going to work right on 32-bits.
Someday V8 will be made 64-bit safe, but that's a much bigger effort
than changing some types around...
On Thu, Jan 22, 2
When we've discussed the v8 pointer/type situation before, it seemed
like there would need to be some discussion on typedefs and style so
that the types could really make sense internally for V8. It's hard,
because there are pointers, pointers get tagged, they have certain
bits used, and a lot of
, wrote:
> On 2009/04/29 09:14:52, Dean McNamee wrote:
>>
>> Just to be annoying, we probably shouldn't be using random() (it's not
>
> thread
>>
>> safe, etc).
>
> Do you have a suggestion on what we should use instead? (Know of a
because V8 is not multi-threaded. I will provide one later this
> week.
>
> Cheers,
> -Ivan
>
>
> On Wed, Apr 29, 2009 at 09:51, Dean McNamee wrote:
>> We solved this for base/rand_util*, although the implementation is
>> more "secure" meaning slow. I am not s
I have a patch out for review. I will clean it up tomorrow. Feel
free to chime in on how you want the naming.
http://codereview.chromium.org/99355
I think something like ARCH_CPU_X64 (I guess we're going with x64 and
not x86-64?), and also ARCH_CPU_32_BITS and ARCH_CPU_64_BITS is good.
I can c
We're hitting a problem with 64-bit, where we can't create long long
literals (like 4LL). We can get away with just doing a long literal
on 64-bit, but it means we can't make a 64-bit literal on 32-bit. We
don't actually need that case just now.
However, it brings up a point. We're using C99 a
Eh, nevermind, for C++ it will be more complicated since C99 isn't in
C++ yet. I will just use a long constant for now :\
Thanks
On Tue, May 5, 2009 at 1:43 PM, Dean McNamee wrote:
> We're hitting a problem with 64-bit, where we can't create long long
> literals (like 4
It would have been found either way if I would have bothered to
compile it, which I didn't, which was sloppy.
On Fri, May 8, 2009 at 8:12 PM, wrote:
>
>
> http://codereview.chromium.org/115022/diff/1/2
> File src/platform-linux.cc (right):
>
> http://codereview.chromium.org/115022/diff/1/2#newc
On Wed, May 20, 2009 at 3:30 PM, Erik Corry wrote:
>
> 2009/5/20 William Hesse :
>> I notice that some functions in the V8 source use the
>> INLINE() macro in their declaration, declared for GCC >=4 as
>>
>> #define INLINE(header) inline header __attribute__((always_inline))
>>
>> and plenty mor
See flag-definitions.h in the V8 source code. You may have to compile
a debug build for some of them.
On Wed, May 27, 2009 at 6:22 PM, silverburgh wrote:
>
>
> In here,
> http://code.google.com/apis/v8/design.html
> it dumps the machine code for 'point.x'.
>
> How can I dump the machine code for
There are several levels of GC that are possible.
I would look at the --expose-gc flag, which is implemented in
GCExtension::GC (calling Heap::CollectAllGarbage()). But there are
other relevant GC functions, see heap.h.
On Fri, May 29, 2009 at 2:15 AM, meryl wrote:
>
> I understand V8 has it ow
See globals.h where there are definitions for V8PRIxPTR, etc.
Also, we probably shouldn't use %p since formatting is different on
different platforms :\
Thanks!
-- dean
On Sat, Jun 13, 2009 at 1:39 AM, Ben Leslie wrote:
>
> Hi all,
>
> In resurrecting nullos support for ARM I came across this c
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
ding a body). I didn't embed bodies to minimize patch and as
> there are some deps on stuff declared below.
>
> Of course, if from stylistic point of view that's preferable to place
> inlines on defs, I'll do that.
>
> yours,
> anton.
>
> On Thu, Jul 23,
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 McNamee wrote:
>>> http://www.parashift.com/c++-faq-lite/inline-functions.html#faq-9.9
>>>
>
; Timur, do you have numbers from the v8 benchmark (release build, not
>> running under valgrind) for before / after?
>>
>> Thanks
>>
>> On 2009/07/20 12:28:51, Dean McNamee wrote:
>>>
>>> Timur reports that this patch (and not having to specify
>>
>&
Yeah, I've seen this too. I was worried in might be a deeper bug.
dtoa is craazzzyyy
On Tue, Aug 18, 2009 at 11:16 AM, wrote:
> Reviewers: Dean McNamee,
>
> Description:
> fta (from Ubuntu) reported a build failure when building v8 with GCC
> 4.4:
> http://paste.ubun
John,
If we pushed out the getTime change, it will be easily to realize what
we've done and quickly adapt to doing something different. Basically
I think it's an ugly hack that will have a small and likely temporary
effect on the real problem.
On Fri, Aug 28, 2009 at 9:00 AM, John Abd-El-Malek
My understand is that interruption works by throwing an exception that
normal JS can't handle. So I'd say if we're calling a handler from
C++, it should be no problem to run code later in that same on
context, and continue running other code in other JS contexts. Mads
should correct me if I'm wr
58 matches
Mail list logo
