[v8-dev] Re: abstract eternal into class (issue 22795004)

2013-08-13 Thread svenpanne
https://chromiumcodereview.appspot.com/22795004/diff/1/include/v8.h File include/v8.h (right): https://chromiumcodereview.appspot.com/22795004/diff/1/include/v8.h#newcode486 include/v8.h:486: templateclass S Why do we have a separate template parameter here? As it is, this signature tells me:

[v8-dev] Re: abstract eternal into class (issue 22795004)

2013-08-13 Thread dcarney
https://chromiumcodereview.appspot.com/22795004/diff/1/include/v8.h File include/v8.h (right): https://chromiumcodereview.appspot.com/22795004/diff/1/include/v8.h#newcode486 include/v8.h:486: templateclass S On 2013/08/13 06:11:18, Sven Panne wrote: Why do we have a separate template parameter

[v8-dev] Ignore flaky intl test. (issue 22853004)

2013-08-13 Thread machenbach
Reviewers: Jakob, Message: PTAL Description: Ignore flaky intl test. BUG= Please review this at https://codereview.chromium.org/22853004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M test/intl/intl.status Index: test/intl/intl.status diff --git

[v8-dev] Caching and reusing Crankshaft optimized code

2013-08-13 Thread yuqiang . xian%intel . com
Hi, We'd like to raise a proposal to cache and reuse the crankshaft optimized code for discussion. We know that V8 currently has the compilation cache with which we can cache and reuse the baseline JIT code, and we also have the shared optimized code for the same CONTEXT. Do we see any

[v8-dev] Re: Ignore flaky intl test. (issue 22853004)

2013-08-13 Thread jkummerow
lgtm https://codereview.chromium.org/22853004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Ignore flaky intl test. (issue 22853004)

2013-08-13 Thread machenbach
Committed patchset #1 manually as r16165. https://codereview.chromium.org/22853004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16165 committed - Ignore flaky intl test....

2013-08-13 Thread codesite-noreply
Revision: 16165 Author: machenb...@chromium.org Date: Tue Aug 13 01:30:06 2013 Log: Ignore flaky intl test. BUG= R=jkumme...@chromium.org Review URL: https://codereview.chromium.org/22853004 http://code.google.com/p/v8/source/detail?r=16165 Modified:

[v8-dev] Re: Added allocation folding support for old space allocations. (issue 22378003)

2013-08-13 Thread hpayer
On 2013/08/09 12:46:08, Michael Starzinger wrote: LGTM. Update: mstarzinger model checker found a bug when old space allocations of different old spaces do not occur in alternating order. Adding the filler map right before the dominating allocate of the other old space should fix that

[v8-dev] Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. (issue 22982003)

2013-08-13 Thread verwaest
Reviewers: danno, Message: PTAL Description: Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. BUG= Please review this at https://chromiumcodereview.appspot.com/22982003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M

[v8-dev] Re: Issue 2839 in v8: Compile time error in natives.h:34:38

2013-08-13 Thread codesite-noreply
Updates: Status: PendingFurtherInfo Comment #2 on issue 2839 by jkumme...@chromium.org: Compile time error in natives.h:34:38 http://code.google.com/p/v8/issues/detail?id=2839 Works for me. Which compilers have you tried? What kind of system are you doing this on? Btw, trunk is

[v8-dev] Refine CountOperation of FullCodeGen (issue 22935005)

2013-08-13 Thread haitao . feng
Reviewers: danno, Jakob, Message: I'd like to know your feedback before porting it to IA32 and ARM. https://codereview.chromium.org/22935005/diff/1/src/x64/full-codegen-x64.cc File src/x64/full-codegen-x64.cc (left):

[v8-dev] Re: Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. (issue 22982003)

2013-08-13 Thread danno
https://codereview.chromium.org/22982003/diff/1/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/22982003/diff/1/src/hydrogen-instructions.h#newcode2558 src/hydrogen-instructions.h:2558: check_map-map_set_.Add(maps-at(i), zone); Why not

[v8-dev] Simplified BuildFastLiteral by eliminating manual allocation folding. (issue 23030002)

2013-08-13 Thread hpayer
Reviewers: Michael Starzinger, Description: Simplified BuildFastLiteral by eliminating manual allocation folding. BUG= Please review this at https://codereview.chromium.org/23030002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/hydrogen.h M

[v8-dev] Re: Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. (issue 22982003)

2013-08-13 Thread verwaest
Addressed comment, ptal again. https://chromiumcodereview.appspot.com/22982003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
I don't think the new implementation actually improves the situation. The case for multiple producers is still not handled correctly. We need to cleanup this whole thing. See my comments below for suggestions how to proceed. CC'ing Sven for additional feedback.

[v8-dev] Re: Introduce StackArgumentsAccessor class for X64 (issue 21123008)

2013-08-13 Thread danno
lgtm if you address the nits, but please wait to land until we've opened the tree again after this week's stability. https://codereview.chromium.org/21123008/diff/49001/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right):

[v8-dev] Run gcmole on i18n code. (issue 23011004)

2013-08-13 Thread jochen
Reviewers: Vyacheslav Egorov, Description: Run gcmole on i18n code. BUG=none R=vego...@chromium.org Please review this at https://codereview.chromium.org/23011004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M tools/gcmole/gcmole.lua Index:

[v8-dev] Re: Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. (issue 22982003)

2013-08-13 Thread danno
lgtm https://chromiumcodereview.appspot.com/22982003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
BTW I did not carefully check the memory-ordering semantics of my suggestions above. https://codereview.chromium.org/22849002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups

[v8-dev] Re: Mark CheckMaps that can cause migration with ChangesNewSpacePromotion. (issue 22982003)

2013-08-13 Thread verwaest
Committed patchset #2 manually as r16166. https://chromiumcodereview.appspot.com/22982003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Introduce SmiValuesAre31Bits and SmiValuesAre32Bits global predicate functions (issue 22350005)

2013-08-13 Thread danno
I like this incremental step, please just change where the predicates are defined as described below and then I think the CL will be ready. https://chromiumcodereview.appspot.com/22350005/diff/1/include/v8.h File include/v8.h (right):

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread yurys
On 2013/08/13 09:31:32, Benedikt Meurer wrote: I don't think the new implementation actually improves the situation. The case for multiple producers is still not handled correctly. We need to cleanup this whole thing. See my comments below for suggestions how to proceed. CC'ing Sven for

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread yurys
https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc File src/circular-queue.cc (right): https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc#newcode81 src/circular-queue.cc:81: MemoryBarrier(); On 2013/08/13 09:31:32, Benedikt Meurer wrote: Why do we

[v8-dev] Re: Run gcmole on i18n code. (issue 23011004)

2013-08-13 Thread vegorov
lgtm https://codereview.chromium.org/23011004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Run gcmole on i18n code. (issue 23011004)

2013-08-13 Thread vegorov
lgtm https://codereview.chromium.org/23011004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Issue 2840 in v8: Implement ES6 String.fromCodePoint, String.prototype.codePointAt, and String.prototype.normalize

2013-08-13 Thread codesite-noreply
Status: New Owner: New issue 2840 by math...@qiwi.be: Implement ES6 String.fromCodePoint, String.prototype.codePointAt, and String.prototype.normalize http://code.google.com/p/v8/issues/detail?id=2840 From ES6 spec (May 2013 revision): * String.fromCodePoint(...codePoints) - section

[v8-dev] Re: Issue 2840 in v8: Implement ES6 String.fromCodePoint, String.prototype.codePointAt, and String.prototype.normalize

2013-08-13 Thread codesite-noreply
Comment #1 on issue 2840 by math...@qiwi.be: Implement ES6 String.fromCodePoint, String.prototype.codePointAt, and String.prototype.normalize http://code.google.com/p/v8/issues/detail?id=2840 FWIW, here’s the same bug ticket for Firefox/Spidermonkey:

[v8-dev] Re: Issue 2436 in v8: Surrogate pairs are not accepted in regular expression character ranges

2013-08-13 Thread codesite-noreply
Comment #1 on issue 2436 by math...@qiwi.be: Surrogate pairs are not accepted in regular expression character ranges http://code.google.com/p/v8/issues/detail?id=2436 Like you said, this is not a V8 bug. IMHO this ticket can be closed. Full Unicode support in regular expressions is

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
On 2013/08/13 10:05:20, Yury Semikhatsky wrote: https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc File src/circular-queue.cc (right): https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc#newcode81 src/circular-queue.cc:81: MemoryBarrier(); On

[v8-dev] Re: Introduce SmiValuesAre31Bits and SmiValuesAre32Bits global predicate functions (issue 22350005)

2013-08-13 Thread haitao . feng
danno, thanks for the review. I will commit this when the tree is open. https://codereview.chromium.org/22350005/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/22350005/diff/1/include/v8.h#newcode5454 include/v8.h:5454: V8_INLINE(static bool

[v8-dev] Re: Introduce PushInt64AsTwoSmis and PopInt64AsTwoSmis macro instructions for X64 (issue 22348005)

2013-08-13 Thread haitao . feng
danno, thanks for the review. I will split them up. I might need to choose a better name for PushInt64AsTwoSmis. It is a general SMI (all the tagged bits are zero). The logic in PushInt64AsTwoSmis works for both kSmiShift=1 and kSmiShift=32, but the value might not be in the range of 31 bit

[v8-dev] Re: Run gcmole on i18n code. (issue 23011004)

2013-08-13 Thread jochen
Committed patchset #1 manually as r16167. https://codereview.chromium.org/23011004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16167 committed - Run gcmole on i18n code....

2013-08-13 Thread codesite-noreply
Revision: 16167 Author: joc...@chromium.org Date: Tue Aug 13 04:05:30 2013 Log: Run gcmole on i18n code. BUG=none R=vego...@chromium.org, vego...@google.com Review URL: https://codereview.chromium.org/23011004 http://code.google.com/p/v8/source/detail?r=16167 Modified:

[v8-dev] Fix gcmole bugs in i18n code (issue 22859006)

2013-08-13 Thread jochen
Reviewers: dcarney, Michael Starzinger, Description: Fix gcmole bugs in i18n code R=mstarzin...@chromium.org,dcar...@chromium.org BUG=v8:2745 Please review this at https://codereview.chromium.org/22859006/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M

[v8-dev] Re: Introduce StackArgumentsAccessor class for X64 (issue 21123008)

2013-08-13 Thread haitao . feng
danno, thanks for the review. I will wait and commit this when the tree is open. https://codereview.chromium.org/21123008/diff/49001/src/x64/code-stubs-x64.cc File src/x64/code-stubs-x64.cc (right): https://codereview.chromium.org/21123008/diff/49001/src/x64/code-stubs-x64.cc#newcode2464

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
On 2013/08/13 10:03:43, Yury Semikhatsky wrote: On 2013/08/13 09:31:32, Benedikt Meurer wrote: I don't think the new implementation actually improves the situation. The case for multiple producers is still not handled correctly. We need to cleanup this whole thing. See my comments below

[v8-dev] Re: Fix gcmole bugs in i18n code (issue 22859006)

2013-08-13 Thread dcarney
lgtm https://codereview.chromium.org/22859006/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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,

[v8-dev] Re: Issue 2839 in v8: Compile time error in natives.h:34:38

2013-08-13 Thread codesite-noreply
Comment #3 on issue 2839 by itbrando...@gmail.com: Compile time error in natives.h:34:38 http://code.google.com/p/v8/issues/detail?id=2839 I have only tried using gcc to compile. My attempts have been on Ubuntu 12.04 LTS. Basically exactly what I have attempted was following the steps

[v8-dev] Improve generalization / migration tracing. (issue 23047002)

2013-08-13 Thread verwaest
Reviewers: Yang, Message: PTAL Description: Improve generalization / migration tracing. Please review this at https://chromiumcodereview.appspot.com/23047002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/objects-inl.h M src/objects.h M

[v8-dev] Merged r16082 into 3.19 branch. (issue 22964003)

2013-08-13 Thread ulan
Reviewers: danno, Description: Merged r16082 into 3.19 branch. Fix Array index dehoisting. BUG=264203 R=da...@chromium.org Please review this at https://chromiumcodereview.appspot.com/22964003/ SVN Base: https://v8.googlecode.com/svn/branches/3.19 Affected files: M src/hydrogen.cc M

[v8-dev] Re: Merged r16082 into 3.19 branch. (issue 22964003)

2013-08-13 Thread danno
lgtm https://chromiumcodereview.appspot.com/22964003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: H-BuildIncrement should make use of available type feedback (issue 22611009)

2013-08-13 Thread verwaest
lgtm https://codereview.chromium.org/22611009/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Issue 2841 in v8: cannot build on Solaris.

2013-08-13 Thread codesite-noreply
Status: New Owner: New issue 2841 by zhega...@enaza.ru: cannot build on Solaris. http://code.google.com/p/v8/issues/detail?id=2841 Solaris 11.0 v8 from git (today). GCC 4.5.2 When building without flags - for some reason Solaris isn't recognized as x64 architecture: # gmake x64

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread yurys
On 2013/08/13 10:28:12, Benedikt Meurer wrote: On 2013/08/13 10:05:20, Yury Semikhatsky wrote: https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc File src/circular-queue.cc (right): https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc#newcode81

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
On 2013/08/13 11:59:53, Yury Semikhatsky wrote: On 2013/08/13 10:28:12, Benedikt Meurer wrote: On 2013/08/13 10:05:20, Yury Semikhatsky wrote: https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc File src/circular-queue.cc (right):

[v8-dev] [v8] r16168 committed - Merged r16082 into 3.19 branch....

2013-08-13 Thread codesite-noreply
Revision: 16168 Author: u...@chromium.org Date: Tue Aug 13 05:02:58 2013 Log: Merged r16082 into 3.19 branch. Fix Array index dehoisting. BUG=264203 R=da...@chromium.org Review URL: https://chromiumcodereview.appspot.com/22964003 http://code.google.com/p/v8/source/detail?r=16168

[v8-dev] Re: Merged r16082 into 3.19 branch. (issue 22964003)

2013-08-13 Thread ulan
Committed patchset #1 manually as r16168 (presubmit successful). https://chromiumcodereview.appspot.com/22964003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group.

[v8-dev] [v8] r16169 committed - Tagging version 3.19.18.19

2013-08-13 Thread codesite-noreply
Revision: 16169 Author: u...@chromium.org Date: Tue Aug 13 05:03:20 2013 Log: Tagging version 3.19.18.19 http://code.google.com/p/v8/source/detail?r=16169 Added: /tags/3.19.18.19 -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You

[v8-dev] Re: Merge 15827, 15842, and 15912 into 3.19 (issue 20767002)

2013-08-13 Thread ulan
On 2013/08/05 09:53:27, danno wrote: Committed patchset #3 manually as r16046 (presubmit successful). This merged did not update version.cc https://codereview.chromium.org/20767002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this

[v8-dev] Re: Introduce PushInt64AsTwoSmis and PopInt64AsTwoSmis macro instructions for X64 (issue 22348005)

2013-08-13 Thread danno
I think PushInt64AsTwoSmis is probably OK, but it's just a little misleading. In the 31-bit case, you shift the 32nd bit up to the 33rd bit of the 64-bit register, so it's no longer part of the smi. However, for the purpose of just saving registers that don't look like pointers, I think it

[v8-dev] Re: Fix gcmole bugs in i18n code (issue 22859006)

2013-08-13 Thread mstarzinger
LGTM. https://codereview.chromium.org/22859006/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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,

[v8-dev] Re: Also add i18n directories to gcmole (issue 22876007)

2013-08-13 Thread vegorov
lgtm https://codereview.chromium.org/22876007/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Also add i18n directories to gcmole (issue 22876007)

2013-08-13 Thread jochen
Reviewers: Vyacheslav Egorov, Description: Also add i18n directories to gcmole R=vego...@chromium.org Please review this at https://codereview.chromium.org/22876007/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M tools/gcmole/gcmole.lua Index:

[v8-dev] [v8] r16170 committed - Also add i18n directories to gcmole...

2013-08-13 Thread codesite-noreply
Revision: 16170 Author: joc...@chromium.org Date: Tue Aug 13 05:22:56 2013 Log: Also add i18n directories to gcmole R=vego...@chromium.org http://code.google.com/p/v8/source/detail?r=16170 Modified: /branches/bleeding_edge/tools/gcmole/gcmole.lua

[v8-dev] [v8] r16171 committed - Fix gcmole bugs in i18n code...

2013-08-13 Thread codesite-noreply
Revision: 16171 Author: joc...@chromium.org Date: Tue Aug 13 05:24:44 2013 Log: Fix gcmole bugs in i18n code R=mstarzin...@chromium.org,dcar...@chromium.org BUG=v8:2745 http://code.google.com/p/v8/source/detail?r=16171 Modified: /branches/bleeding_edge/src/i18n.cc

[v8-dev] Re: Refine CountOperation of FullCodeGen (issue 22935005)

2013-08-13 Thread danno
Although I don't see anything wrong with this patch, I am unclear of the motivation. Is there a clear performance win by eliminating the eliminating the duplicate smi check? Have you tried to quantify the improvement and is it measurable? If not, I suspect that having more compact code rather

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread danno
https://codereview.chromium.org/22152003/diff/17024/src/hydrogen-mark-deoptimize.cc File src/hydrogen-mark-deoptimize.cc (right): https://codereview.chromium.org/22152003/diff/17024/src/hydrogen-mark-deoptimize.cc#newcode74 src/hydrogen-mark-deoptimize.cc:74: if

[v8-dev] Re: Move ToI conversions to the MacroAssembler (issue 22290005)

2013-08-13 Thread danno
https://codereview.chromium.org/22290005/diff/44001/src/ia32/macro-assembler-ia32.cc File src/ia32/macro-assembler-ia32.cc (right): https://codereview.chromium.org/22290005/diff/44001/src/ia32/macro-assembler-ia32.cc#newcode281 src/ia32/macro-assembler-ia32.cc:281: XMMRegister temp, bool

[v8-dev] Alias HStoreNamedField::observed_input_representation to its required input representation. (issue 23059002)

2013-08-13 Thread verwaest
Reviewers: Jakob, Message: PTAL Description: Alias HStoreNamedField::observed_input_representation to its required input representation. BUG= Please review this at https://chromiumcodereview.appspot.com/23059002/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files:

[v8-dev] Re: Introduce PushInt64AsTwoSmis and PopInt64AsTwoSmis macro instructions for X64 (issue 22348005)

2013-08-13 Thread haitao . feng
Thanks for the comment. I have made this CL to include the PushInt64AsTwoSmis only. I added a comment in the header file, the comment is a little misleading also before 31-bit SMI is not introduced as currently they are exactly SMIs. https://codereview.chromium.org/22348005/ -- -- v8-dev

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread verwaest
Addressed comment https://chromiumcodereview.appspot.com/22152003/diff/17024/src/hydrogen-mark-deoptimize.cc File src/hydrogen-mark-deoptimize.cc (right): https://chromiumcodereview.appspot.com/22152003/diff/17024/src/hydrogen-mark-deoptimize.cc#newcode74 src/hydrogen-mark-deoptimize.cc:74: if

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread danno
lgtm https://chromiumcodereview.appspot.com/22152003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Refine CountOperation of FullCodeGen (issue 22935005)

2013-08-13 Thread haitao . feng
Sorry I should make the motivation clear. I am addressing your comment at https://codereview.chromium.org/21014003/patch/17001/18010. When I used the passing the stub-call label always approach for 31-bit SMI, I found 33 fails for this part of code as the SmiAddConstant and SmiSubConstant in

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread verwaest
Ported to ARM / X64, PTAL https://chromiumcodereview.appspot.com/22152003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread mstarzinger
https://codereview.chromium.org/22545007/diff/20001/src/array.js File src/array.js (right): https://codereview.chromium.org/22545007/diff/20001/src/array.js#newcode1263 src/array.js:1263: if (!isProxy %HasFastPackedElements(array) || i in array) { It is _really_ surprising that the runtime

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread yurys
Benedikt, please take another look. https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc File src/circular-queue.cc (right): https://codereview.chromium.org/22849002/diff/2001/src/circular-queue.cc#newcode94 src/circular-queue.cc:94: } On 2013/08/13 09:31:32, Benedikt

[v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread mstarzinger
https://codereview.chromium.org/22545007/diff/20001/src/array.js File src/array.js (right): https://codereview.chromium.org/22545007/diff/20001/src/array.js#newcode1315 src/array.js:1315: if (!IS_UNDEFINED(element) || i in array) { On 2013/08/13 14:10:22, Michael Starzinger wrote: This is

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread bmeurer
Ok, next round of comments. https://codereview.chromium.org/22849002/diff/23001/src/allocation.h File src/allocation.h (right): https://codereview.chromium.org/22849002/diff/23001/src/allocation.h#newcode39 src/allocation.h:39: // ALIGNAS(16) int array[4]; Typo: ALIGN_AT

[v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread arv
On 2013/08/13 14:10:21, Michael Starzinger wrote: https://codereview.chromium.org/22545007/diff/20001/src/array.js File src/array.js (right): https://codereview.chromium.org/22545007/diff/20001/src/array.js#newcode1263 src/array.js:1263: if (!isProxy %HasFastPackedElements(array) || i in

[v8-dev] Store doubles before calling into the elements transition stub on ARM (issue 22854011)

2013-08-13 Thread verwaest
Reviewers: ulan, Message: PTAL Description: Store doubles before calling into the elements transition stub on ARM BUG= Please review this at https://chromiumcodereview.appspot.com/22854011/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M

[v8-dev] Re: Store doubles before calling into the elements transition stub on ARM (issue 22854011)

2013-08-13 Thread ulan
lgtm https://chromiumcodereview.appspot.com/22854011/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Store doubles before calling into the elements transition stub on ARM (issue 22854011)

2013-08-13 Thread verwaest
Committed patchset #2 manually as r16172. https://chromiumcodereview.appspot.com/22854011/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16172 committed - Store doubles before calling into the elements transition stub on ARM...

2013-08-13 Thread codesite-noreply
Revision: 16172 Author: verwa...@chromium.org Date: Tue Aug 13 08:06:17 2013 Log: Store doubles before calling into the elements transition stub on ARM BUG= R=u...@chromium.org Review URL: https://chromiumcodereview.appspot.com/22854011

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread rodolph . perfetta
dbc https://chromiumcodereview.appspot.com/22152003/diff/35001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://chromiumcodereview.appspot.com/22152003/diff/35001/src/arm/lithium-codegen-arm.cc#newcode2161 src/arm/lithium-codegen-arm.cc:2161: void

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread verwaest
Hi Rodolph, thanks for the DBC! I have an inline question. https://chromiumcodereview.appspot.com/22152003/diff/35001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right):

[v8-dev] Re: Never hchange nan-hole to hole or hole to nan-hole. (issue 22152003)

2013-08-13 Thread rodolph . perfetta
https://chromiumcodereview.appspot.com/22152003/diff/35001/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://chromiumcodereview.appspot.com/22152003/diff/35001/src/arm/lithium-codegen-arm.cc#newcode2161 src/arm/lithium-codegen-arm.cc:2161: void

[v8-dev] Re: Rewrite SamplingCircularQueue (issue 22849002)

2013-08-13 Thread yurys
https://codereview.chromium.org/22849002/diff/23001/src/circular-queue.h File src/circular-queue.h (right): https://codereview.chromium.org/22849002/diff/23001/src/circular-queue.h#newcode78 src/circular-queue.h:78: Atomic32 marker ALIGN_AT(sizeof(Atomic32)); On 2013/08/13 14:52:37, Yury

[v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread mstarzinger
https://codereview.chromium.org/22545007/diff/20001/src/array.js File src/array.js (right): https://codereview.chromium.org/22545007/diff/20001/src/array.js#newcode1263 src/array.js:1263: if (!isProxy %HasFastPackedElements(array) || i in array) { On 2013/08/13 14:10:22, Michael Starzinger

Re: [v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread Vyacheslav Egorov
s/%HasElement(i, array)/%HasElement(array, i)/ Vyacheslav Egorov On Tue, Aug 13, 2013 at 6:04 PM, Vyacheslav Egorov vego...@chromium.orgwrote: DBC: Can't we just manually inline 'in' here? if I am not mistaken x in y essentially just calls IN function from runtime.js In this

Re: [v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread Vyacheslav Egorov
DBC: Can't we just manually inline 'in' here? if I am not mistaken x in y essentially just calls IN function from runtime.js In this particular case you should be able just to replace i in array with %HasElement(i, array) and hopefully fast path in the %HasElement would take care of the rest.

[v8-dev] Make HToFastProperties GC safe. (issue 22980003)

2013-08-13 Thread jkummerow
Reviewers: Michael Starzinger, Message: PTAL. Description: Make HToFastProperties GC safe. The runtime call can cause a GC, so the instruction must have proper flags set. Please review this at https://codereview.chromium.org/22980003/ SVN Base:

[v8-dev] Expect and check for dependency change during code generation. (issue 23064003)

2013-08-13 Thread yangguo
Reviewers: Jakob, Description: Expect and check for dependency change during code generation. R=jkumme...@chromium.org BUG= Please review this at https://codereview.chromium.org/23064003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/compiler.cc

[v8-dev] Re: Expect and check for dependency change during code generation. (issue 23064003)

2013-08-13 Thread yangguo
On 2013/08/13 16:20:33, Yang wrote: arm.debug tests are triggering the assertion that I removed. Instead, the check should guarantee that we abort compilation. https://codereview.chromium.org/23064003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev

[v8-dev] Re: Make HToFastProperties GC safe. (issue 22980003)

2013-08-13 Thread mstarzinger
LGTM with a comment. https://codereview.chromium.org/22980003/diff/3001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/22980003/diff/3001/src/hydrogen-instructions.h#newcode6553 src/hydrogen-instructions.h:6553:

[v8-dev] Re: Expect and check for dependency change during code generation. (issue 23064003)

2013-08-13 Thread jkummerow
LGTM. Can we have a platform-independent repro case? A follow-up CL is fine. https://codereview.chromium.org/23064003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev

[v8-dev] Re: Make HToFastProperties GC safe. (issue 22980003)

2013-08-13 Thread jkummerow
Committed patchset #3 manually as r16173. https://codereview.chromium.org/22980003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16173 committed - Make HToFastProperties GC safe....

2013-08-13 Thread codesite-noreply
Revision: 16173 Author: jkumme...@chromium.org Date: Tue Aug 13 09:26:53 2013 Log: Make HToFastProperties GC safe. The runtime call can cause a GC, so the instruction must have proper flags set. R=mstarzin...@chromium.org Review URL: https://codereview.chromium.org/22980003

[v8-dev] Fix bug in HPhi::SimplifyConstantInput (issue 23075003)

2013-08-13 Thread yangguo
Reviewers: Jakob, Description: Fix bug in HPhi::SimplifyConstantInput R=jkumme...@chromium.org BUG=269679 Please review this at https://codereview.chromium.org/23075003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/hydrogen-instructions.h M

[v8-dev] Re: Fix bug in HPhi::SimplifyConstantInput (issue 23075003)

2013-08-13 Thread jkummerow
lgtm https://codereview.chromium.org/23075003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Remove dead JSReceiver::SetElement method. (issue 22806004)

2013-08-13 Thread mstarzinger
Reviewers: rossberg, Description: Remove dead JSReceiver::SetElement method. R=rossb...@chromium.org Please review this at https://codereview.chromium.org/22806004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M src/objects.h M src/objects.cc Index:

[v8-dev] Re: Fix bug in HPhi::SimplifyConstantInput (issue 23075003)

2013-08-13 Thread yangguo
Committed patchset #1 manually as r16174. https://codereview.chromium.org/23075003/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16174 committed - Fix bug in HPhi::SimplifyConstantInput...

2013-08-13 Thread codesite-noreply
Revision: 16174 Author: yang...@chromium.org Date: Tue Aug 13 09:47:27 2013 Log: Fix bug in HPhi::SimplifyConstantInput R=jkumme...@chromium.org BUG=269679 Review URL: https://codereview.chromium.org/23075003 http://code.google.com/p/v8/source/detail?r=16174 Added:

[v8-dev] Use Cell instead of PropertyCell in DoCheckFunction (in case of new space object). (issue 23036004)

2013-08-13 Thread yangguo
Reviewers: danno, Description: Use Cell instead of PropertyCell in DoCheckFunction (in case of new space object). R=da...@chromium.org BUG= Please review this at https://codereview.chromium.org/23036004/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M

[v8-dev] Re: Use Cell instead of PropertyCell in DoCheckFunction (in case of new space object). (issue 23036004)

2013-08-13 Thread danno
lgtm https://codereview.chromium.org/23036004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] [v8] r16175 committed - Use Cell instead of PropertyCell in DoCheckFunction (in case of new sp...

2013-08-13 Thread codesite-noreply
Revision: 16175 Author: yang...@chromium.org Date: Tue Aug 13 09:58:14 2013 Log: Use Cell instead of PropertyCell in DoCheckFunction (in case of new space object). R=da...@chromium.org BUG= Review URL: https://codereview.chromium.org/23036004

[v8-dev] Re: Use Cell instead of PropertyCell in DoCheckFunction (in case of new space object). (issue 23036004)

2013-08-13 Thread yangguo
Committed patchset #1 manually as r16175. https://codereview.chromium.org/23036004/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Expect and check for dependency change during code generation. (issue 23064003)

2013-08-13 Thread yangguo
On 2013/08/13 16:22:23, Jakob wrote: LGTM. Can we have a platform-independent repro case? A follow-up CL is fine. Fixed it in https://codereview.chromium.org/23036004/ https://codereview.chromium.org/23064003/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Prepare push to trunk. Now working on version 3.20.17. (issue 23073003)

2013-08-13 Thread danno
Reviewers: Jakob, Description: Prepare push to trunk. Now working on version 3.20.17. R=jkumme...@chromium.org Please review this at https://codereview.chromium.org/23073003/ SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files: M ChangeLog M src/version.cc

[v8-dev] Re: Prepare push to trunk. Now working on version 3.20.17. (issue 23073003)

2013-08-13 Thread jkummerow
LGTM with comment. https://codereview.chromium.org/23073003/diff/1/ChangeLog File ChangeLog (right): https://codereview.chromium.org/23073003/diff/1/ChangeLog#newcode21 ChangeLog:21: CompoundAssignment. (Chromium issue 2774,2779) These are not Chromium issues.

Re: [v8-dev] Re: Hack forEach to use HasFastPackedElements (issue 22545007)

2013-08-13 Thread Erik Arvidsson
HasElements is 21% faster than existing code and it still passes all tests. I'll provide a new CL that only does that. On Tue, Aug 13, 2013 at 12:05 PM, Vyacheslav Egorov vego...@chromium.orgwrote: s/%HasElement(i, array)/%HasElement(array, i)/ Vyacheslav Egorov On Tue, Aug 13, 2013 at

  1   2   >