[v8-dev] Re: Implement parsing ES6 Template Literals (issue 663683006 by caitpotte...@gmail.com)
LGTM https://codereview.chromium.org/663683006/diff/540001/test/mjsunit/harmony/templates.js File test/mjsunit/harmony/templates.js (right): https://codereview.chromium.org/663683006/diff/540001/test/mjsunit/harmony/templates.js#newcode297 test/mjsunit/harmony/templates.js:297: assertTrue(cs.raw instanceof Array); Similar issue with instanceof as in. In test you are better of being more precise: assertEquals(Array.prototype. Object.getPrototypeOf(cs.raw)); assertTrue(Array.isArray(cs.raw)); You do not really have to change this but next time you write tests keep it in mind. https://codereview.chromium.org/663683006/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Move public symbols to the root set. (issue 722723002 by yang...@chromium.org)
https://codereview.chromium.org/722723002/diff/20001/src/ast-value-factory.cc File src/ast-value-factory.cc (right): https://codereview.chromium.org/722723002/diff/20001/src/ast-value-factory.cc#newcode185 src/ast-value-factory.cc:185: DCHECK_EQ(0, strcmp(symbol_name_, iterator_symbol)); On 2014/11/12 16:45:25, rossberg wrote: Why can this only happen for the iterator symbol? right now this is only used for the iterator symbol. if we need other symbols embedded in the future, we would have to multiplex. https://codereview.chromium.org/722723002/diff/20001/src/bootstrapper.cc File src/bootstrapper.cc (right): https://codereview.chromium.org/722723002/diff/20001/src/bootstrapper.cc#newcode1994 src/bootstrapper.cc:1994: JSObject::AddProperty(builtins, varname, factory()-name(), attributes); This installs the public symbols on the builtins object. https://codereview.chromium.org/722723002/diff/20001/src/harmony-tostring.js File src/harmony-tostring.js (left): https://codereview.chromium.org/722723002/diff/20001/src/harmony-tostring.js#oldcode12 src/harmony-tostring.js:12: var symbolToStringTag = InternalSymbol(Symbol.toStringTag); On 2014/11/12 16:45:25, rossberg wrote: Hm, I'm confused. Where is this variable defined now? Note that it is used below. See bootstrapper.cc https://codereview.chromium.org/722723002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #2 on issue 3692 by mobilebr...@gmail.com: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 I found that if I change the _beget method to delegate to another function, the failure also goes away without having to introduce a try/finally. for example: ```js Promise.prototype._beget = function() { return begetFrom(this._handler, this.constructor); }; function begetFrom(parent, Promise) { var child = new Pending(parent.receiver, parent.join().context); return new Promise(Handler, child); } ``` Which is functionally equivalent to the original, but perhaps changes the optimized bytecode output? -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Move public symbols to the root set. (issue 722723002 by yang...@chromium.org)
LGTM. Some of the stuff (for_intern field) in runtime-symbols.cc is probably obsolete now, though. https://codereview.chromium.org/722723002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #3 on issue 3692 by i...@bnoordhuis.nl: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 @OP: node.js v0.10 ships with V8 3.14, which is no longer supported by upstream. For that matter, the latest v0.11 release bundles V8 3.26, which isn't supported anymore, either. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #4 on issue 3692 by mobilebr...@gmail.com: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 Is there another party, besides upstream who is supporting those versions of v8? -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Make codereview.settings ready for git. (issue 699153003 by machenb...@chromium.org)
Committed patchset #1 (id:1) manually as e7622993c5c27878ffdc6e11a30b8d1f320a4536 (tree was closed). https://codereview.chromium.org/699153003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #5 on issue 3692 by mobilebr...@gmail.com: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 Sorry, a more productive question probably is: Who supports those versions of v8? The node team? I'm happy to file this issue with the appropriate folks if this isn't the right place. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Switch release scripts to pure git. (issue 716153002 by machenb...@chromium.org)
Committed patchset #3 (id:40001) manually as 90b6fc7f1cd0bc3bd5f099e6cdb83d01960da45d (tree was closed). https://codereview.chromium.org/716153002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove unused dot_for and dot_result strings from heap.h (issue 698483004 by ad...@chromium.org)
Committed patchset #1 (id:1) manually as 5f7b24f7b42950f81a609d0eb82a9090751ac2dd (presubmit successful). https://codereview.chromium.org/698483004/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re-add dot_result_string to heap.h after 5f7b24f7b42 (issue 720793004 by ad...@chromium.org)
Reviewers: marja, Message: Committed patchset #1 (id:1) manually as c93c8969d1c9fbc6f926b9760cb3a230e644ea41. Description: Re-add dot_result_string to heap.h after 5f7b24f7b42 Failed to notice it was still being used in a DCHECK, so removing it broke the debug build. TBR=ma...@chromium.org Committed: https://chromium.googlesource.com/v8/v8/+/c93c8969d1c9fbc6f926b9760cb3a230e644ea41 Please review this at https://codereview.chromium.org/720793004/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+2, -1 lines): M include/v8.h M src/heap/heap.h Index: include/v8.h diff --git a/include/v8.h b/include/v8.h index 26435a85b6a1bcda7fa5d97465d0002826c52c00..e2dee5d7b3007d9e525fec11f1bbbd94ccf49a97 100644 --- a/include/v8.h +++ b/include/v8.h @@ -6118,7 +6118,7 @@ class Internals { static const int kNullValueRootIndex = 7; static const int kTrueValueRootIndex = 8; static const int kFalseValueRootIndex = 9; - static const int kEmptyStringRootIndex = 153; + static const int kEmptyStringRootIndex = 154; // The external allocation limit should be below 256 MB on all architectures // to avoid that resource-constrained embedders run low on memory. Index: src/heap/heap.h diff --git a/src/heap/heap.h b/src/heap/heap.h index aa7e1acffdaebc4ca7f058aa32080e00e6d25716..e292c322bac8a6e73fb61cc206f202d332a887fb 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -209,6 +209,7 @@ namespace internal { V(Boolean_string, Boolean) \ V(callee_string, callee) \ V(constructor_string, constructor) \ + V(dot_result_string, .result) \ V(eval_string, eval) \ V(empty_string, ) \ V(function_string, function) \ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)
On 2014/11/12 14:49:27, dcarney wrote: lgtm, a simple unit test is really all you can do here Yeah. And I'm having difficulty with changing the command line flags in cctest. If I can't resolve this tomorrow, I'll commit without unit test. https://codereview.chromium.org/718043002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)
https://codereview.chromium.org/718043002/diff/20001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/718043002/diff/20001/src/api.cc#newcode1851 src/api.cc:1851: internal::CpuFeatures::SupportedFeatures(); On 2014/11/12 15:55:31, Sven Panne wrote: This is a very bad hash, please use the hashing facilitites from src/base/functional.h Done. Thanks; I was looking for something like those hash functions, but somehow didn't find them. :( https://codereview.chromium.org/718043002/diff/20001/src/flags.cc File src/flags.cc (right): https://codereview.chromium.org/718043002/diff/20001/src/flags.cc#newcode553 src/flags.cc:553: uint32_t FlagList::Hash() { On 2014/11/12 15:55:31, Sven Panne wrote: Again, src/base/functional.h might help here Done. https://codereview.chromium.org/718043002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Avoid fast short-cut in Map::GeneralizeRepresentation() for literals with non-simple transitions. (issue 715313003 by ish...@chromium.org)
Reviewers: Toon Verwaest, Message: PTAL Description: Avoid fast short-cut in Map::GeneralizeRepresentation() for literals with non-simple transitions. It started showing after r25253. BUG=v8:3687 Please review this at https://codereview.chromium.org/715313003/ Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+22, -12 lines): M src/objects.cc A + test/mjsunit/regress/regress-3687.js Index: src/objects.cc diff --git a/src/objects.cc b/src/objects.cc index 01b4a1aca09f61e9504b67a41f7f6abb66971e39..e6c46432dc5eee76b216f7d791751af2d0feb68b 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -2471,9 +2471,13 @@ HandleMap Map::GeneralizeRepresentation(HandleMap old_map, // modification to the object, because the default uninitialized value for // representation None can be overwritten by both smi and tagged values. // Doubles, however, would require a box allocation. - if (old_representation.IsNone() - !new_representation.IsNone() - !new_representation.IsDouble()) { + // Note that if the map has transitions that do not share descriptors we + // can't use this shortcut because field type and representation must be + // updated in all the transitions from this map. + if (old_representation.IsNone() !new_representation.IsNone() + !new_representation.IsDouble() + (!old_map-HasTransitionArray() || + old_map-transitions()-IsSimpleTransition())) { DCHECK(old_details.type() == FIELD); DCHECK(old_descriptors-GetFieldType(modify_index)-NowIs( HeapType::None())); Index: test/mjsunit/regress/regress-3687.js diff --git a/test/mjsunit/regress/regress-crbug-357330.js b/test/mjsunit/regress/regress-3687.js similarity index 56% copy from test/mjsunit/regress/regress-crbug-357330.js copy to test/mjsunit/regress/regress-3687.js index b3edf00843e1a9d202212c24d96dc3ad5d027f12..e1df1b4e1d3dbef376a13a2e80e7fb543ee639b9 100644 --- a/test/mjsunit/regress/regress-crbug-357330.js +++ b/test/mjsunit/regress/regress-3687.js @@ -4,13 +4,19 @@ // Flags: --allow-natives-syntax -function f(foo) { - var g; - true ? (g = foo + 0) : g = null; - if (null != g) {} -}; +var t1 = { f1: 0 }; +var t2 = { f2: 0 }; -f(1.4); -f(1.4); -%OptimizeFunctionOnNextCall(f); -f(1.4); +var z = { + x: { +x: t1, +y: { + x: {}, + z1: { +x: t2, +y: 1 + } +} + }, + z2: 0 +}; -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #6 on issue 3692 by i...@bnoordhuis.nl: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 Who supports those versions of v8? The node team? Sadly, no. It's a bug in the current development process. Sometimes, it's possible to cherry-pick patches from upstream but only if they are small and apply cleanly. In general, however, you should not expect any fixes to be forthcoming. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Remove dead AST code in For and While statements (issue 717923003 by ad...@chromium.org)
Committed patchset #1 (id:1) manually as d7f8ea2c68bc03ad794633af5f3453e669fd6e83 (presubmit successful). https://codereview.chromium.org/717923003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)
lgtm 2 Now it's clear from the function name that this is not to be used as the cache tag as is, but that it merely provides the version part of it. I'm not sure what kind of unit test would add value - I'm okay with adding none. The test would basically need to be such that it supports making code changes in this area.. and I think the most probable code change will be that we figure out oops, XYZ should've also been taken into account in the cache tag, and there's no test to tell us that. Tests like tests that the tag changes if the flag change are mildly useful. https://codereview.chromium.org/718043002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Issue 3693 in v8: Cannot build Android target from OSX host
Status: New Owner: New issue 3693 by jsha...@gmail.com: Cannot build Android target from OSX host https://code.google.com/p/v8/issues/detail?id=3693 Running 'make builddeps' succeeded. Then running 'make android_arm.release -j16' failed with the following: CXX(host) /Users/jshagam/src/v8/out/android_arm.release/obj.host/icui18n/third_party/icu/source/i18n/msgfmt.o error: invalid argument '-std=gnu++0x' not allowed with 'C/ObjC' make[2]: *** [/Users/jshagam/src/v8/out/android_arm.release/obj.host/icui18n/third_party/icu/source/i18n/decContext.o] Error 1 make[2]: *** Waiting for unfinished jobs error: invalid argument '-std=gnu++0x' not allowed with 'C/ObjC' make[2]: *** [/Users/jshagam/src/v8/out/android_arm.release/obj.host/icui18n/third_party/icu/source/i18n/decNumber.o] Error 1 make[1]: *** [android_arm.release] Error 2 make: *** [android_arm.release] Error 2 This is from V8 checked out from git master at id d543c9df39e24f4cb46e087f47511a89b71e4bcd. From the error I'm guessing that the build system is trying to compile vanilla C sources with C++0x support, which the compiler does not like. OS: OSX Mavericks (10.9.5) XCode version: Version 6.1 (6A1052d) Host compiler: Apple LLVM version 6.0 (clang-600.0.54) (based on LLVM 3.5svn) Android NDK version: r10c This also occurs in the 3.30 branch. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Add a version tag for cached data. (issue 718043002 by vogelh...@chromium.org)
On Wed, Nov 12, 2014 at 7:48 PM, ma...@chromium.org wrote: lgtm 2 Now it's clear from the function name that this is not to be used as the cache tag as is, but that it merely provides the version part of it. I'm not sure what kind of unit test would add value - I'm okay with adding none. The test would basically need to be such that it supports making code changes in this area.. and I think the most probable code change will be that we figure out oops, XYZ should've also been taken into account in the cache tag, and there's no test to tell us that. Tests like tests that the tag changes if the flag change are mildly useful. Yeah. The test I had is essentially: force --some-flag=true; get tag1; force --some-flag=false; get tag2; CHECK(tag1 != tag2). Since this checks only one of three sources of tag differences it's a pretty random unit test. At least a broken test would definitely imply broken code, but clearly not vice versa. I'll be most happy to leave it out. -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #7 on issue 3692 by mobilebr...@gmail.com: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 Thanks for the candid answer. Since this problem is manifesting via the when.js package, as the maintainer, it seems I have no real recourse but to try to devise a workaround (like the one above that splits _beget), and cross my fingers that the problem no longer manifests in applications that depend on the package. That's a pretty frustrating situation for me, and for users of when.js. To end on a better note, node 0.11.14 hasn't yet hit the same failure with the test program. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Issue 3694 in v8: Segmentation fault in CopyCharsUnsigned() with g++ 4.9.2
Status: New Owner: New issue 3694 by i...@bnoordhuis.nl: Segmentation fault in CopyCharsUnsigned() with g++ 4.9.2 https://code.google.com/p/v8/issues/detail?id=3694 The current implementation of CopyCharsUnsigned() in src/utils.h makes x86_64 g++ 4.9.2 emit movdqa instructions for unaligned addresses. I'm not entirely sure if g++ is to blame here or if there is a pointer aliasing bug in V8 but replacing this line: while (dest limit) *dest++ = static_castsinkchar(*src++); With a byte-for-byte copy fixes the issue. I've only seen the SIGSEGV happen in calls to Factory::NewStringFromTwoByte() and only with a source and destination of type uint16_t. Tested with the current HEAD and v3.29.93.1. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Avoid fast short-cut in Map::GeneralizeRepresentation() for literals with non-simple transitions. (issue 715313003 by ish...@chromium.org)
Thanks for finding the problem! https://codereview.chromium.org/715313003/diff/1/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/715313003/diff/1/src/objects.cc#newcode2480 src/objects.cc:2480: old_map-transitions()-IsSimpleTransition())) { What about instead updating the fieldtype in the entire tree? There's infrastructure to do that. Knowing we have a simple transition doesn't guarantee (I believe) that we are sharing the descriptor array... https://codereview.chromium.org/715313003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Classes: Implement correct name binding (issue 722793005 by a...@chromium.org)
Reviewers: adamk, Dmitry Lomov (chromium), Message: PTAL https://codereview.chromium.org/722793005/diff/1/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/722793005/diff/1/src/parser.cc#newcode3922 src/parser.cc:3922: ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name, I had to split the code from ParserBase into Parser and PreParser specific implementations :'( The parser base cannot handle variables and bindings. On the plus side, it means that the pre parser got simpler. Description: Classes: Implement correct name binding Named class declarations and class expression have a const binding for the name that is in TDZ for the extends expression. BUG=v8:3330 LOG=Y R=dslo...@chromium.org, adamk Please review this at https://codereview.chromium.org/722793005/ Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge Affected files (+300, -116 lines): M src/ast.h M src/ast-numbering.cc M src/full-codegen.cc M src/parser.h M src/parser.cc M src/preparser.h M src/preparser.cc M test/mjsunit/harmony/classes.js -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3692 in v8: function suddenly becomes undefined
Comment #8 on issue 3692 by m.go...@gmail.com: function suddenly becomes undefined https://code.google.com/p/v8/issues/detail?id=3692 Re #6: Who supports those versions of v8? The node team? Sadly, no. It's a bug in the current development process. Wow, that is sad. Considering the importance of Node I think Google should evaluate the concept of v8 LTS releases. I don't mean the span from Node 0.10 to 0.12, it is way too long but at least a few months perhaps? Either that or Node should be able to update v8 each 6 weeks, even on stable releases. Current situation is really scary for Node public apps. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3683 in v8: C-style for-let can't handle continue
Updates: Owner: --- Cc: ad...@chromium.org Comment #14 on issue 3683 by ad...@chromium.org: C-style for-let can't handle continue https://code.google.com/p/v8/issues/detail?id=3683 I've solved the label issue (basically by avoiding labelling the outer for statement at all: I just pass it in to the break statements that target it). I can now pass the test cases at the top of this bug. But I think we have a bigger problem: this desugaring doesn't work. The trouble is that the next stuff runs in the wrong scope: it affects the inner x, meaning closures that capture x the wrong value. Basically, it has the problem Allen describes in https://mail.mozilla.org/pipermail/es-discuss/2014-June/037770.html Now thinking hard about alternatives... (note: still owning this, just unassigning myself due to a codesite bug) -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Classes: Implement correct name binding (issue 722793005 by a...@chromium.org)
Just a style point, I don't yet feel comfortable enough with the parser to offer much insight on this patch. https://codereview.chromium.org/722793005/diff/1/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/722793005/diff/1/src/parser.cc#newcode3932 src/parser.cc:3932: if (this-IsEvalOrArguments(name)) { I don't think you need these explicit |this| references now that your code is in {,pre}parser.cc Same goes everywhere below where this- appears. https://codereview.chromium.org/722793005/diff/1/src/preparser.cc File src/preparser.cc (right): https://codereview.chromium.org/722793005/diff/1/src/preparser.cc#newcode946 src/preparser.cc:946: return this-EmptyExpression(); this- unnecessary here and below, as well. https://codereview.chromium.org/722793005/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3683 in v8: C-style for-let can't handle continue
Comment #15 on issue 3683 by ad...@chromium.org: C-style for-let can't handle continue https://code.google.com/p/v8/issues/detail?id=3683 (in case you want to check my work, work-in-progress patch at https://codereview.chromium.org/720863002/; I think it's mostly-correct except for position information). -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] X87: Leaving a generator via an exception causes it to close (issue 724643002 by chunyang....@intel.com)
Reviewers: Weiliang, wingo, Message: WL. Please review. thanks. Description: X87: Leaving a generator via an exception causes it to close port 24a7ee877e1fe2ba0a52d69061946aeda7c26c10 (r25297) original commit message: Leaving a generator via an exception causes it to close BUG= Please review this at https://codereview.chromium.org/724643002/ Base URL: https://chromium.googlesource.com/external/v8.git@bleeding_edge Affected files (+1, -29 lines): M src/x87/full-codegen-x87.cc Index: src/x87/full-codegen-x87.cc diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc index 8ed9ff0f8125602e276c6efeac4f9f849c382cc9..fa9088cdf549e3c0e4220fb6ce3581552f648dc6 100644 --- a/src/x87/full-codegen-x87.cc +++ b/src/x87/full-codegen-x87.cc @@ -2112,15 +2112,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, VisitForAccumulatorValue(value); __ pop(ebx); - // Check generator state. - Label wrong_state, closed_state, done; - STATIC_ASSERT(JSGeneratorObject::kGeneratorExecuting 0); - STATIC_ASSERT(JSGeneratorObject::kGeneratorClosed == 0); - __ cmp(FieldOperand(ebx, JSGeneratorObject::kContinuationOffset), - Immediate(Smi::FromInt(0))); - __ j(equal, closed_state); - __ j(less, wrong_state); - // Load suspended function and context. __ mov(esi, FieldOperand(ebx, JSGeneratorObject::kContextOffset)); __ mov(edi, FieldOperand(ebx, JSGeneratorObject::kFunctionOffset)); @@ -2142,7 +2133,7 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Enter a new JavaScript frame, and initialize its slots as they were when // the generator was suspended. - Label resume_frame; + Label resume_frame, done; __ bind(push_frame); __ call(resume_frame); __ jmp(done); @@ -2189,25 +2180,6 @@ void FullCodeGenerator::EmitGeneratorResume(Expression *generator, // Not reached: the runtime call returns elsewhere. __ Abort(kGeneratorFailedToResume); - // Reach here when generator is closed. - __ bind(closed_state); - if (resume_mode == JSGeneratorObject::NEXT) { -// Return completed iterator result when generator is closed. -__ push(Immediate(isolate()-factory()-undefined_value())); -// Pop value from top-of-stack slot; box result into result register. -EmitCreateIteratorResult(true); - } else { -// Throw the provided value. -__ push(eax); -__ CallRuntime(Runtime::kThrow, 1); - } - __ jmp(done); - - // Throw error if we attempt to operate on a running generator. - __ bind(wrong_state); - __ push(ebx); - __ CallRuntime(Runtime::kThrowGeneratorStateError, 1); - __ bind(done); context()-Plug(result_register()); } -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Fix the paged space statistics report error under debug mode. (issue 725453002 by chunyang....@intel.com)
Reviewers: danno, Message: hello. Danno. PTAL. thanks. Description: Fix the paged space statistics report error under debug mode. If the code event handler is set (enable vtune jit or gdb jit). GC will be triggered in Logger::LogCodeObjects during Isolate initializaiton. But the property_cell_space_used_ definition in snapshot is 0. This leads to the /0 error if --heap_stats flag is enabled. BUG= Please review this at https://codereview.chromium.org/725453002/ Base URL: https://chromium.googlesource.com/external/v8.git@bleeding_edge Affected files (+2, -1 lines): M src/heap/spaces.cc Index: src/heap/spaces.cc diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc index 2b696ea8eb5238f91f5ade5702cc663f4c09cc73..aa244ad94a1f86d27a148e6a8c484173b31af9da 100644 --- a/src/heap/spaces.cc +++ b/src/heap/spaces.cc @@ -2791,7 +2791,8 @@ void PagedSpace::CollectCodeStatistics() { void PagedSpace::ReportStatistics() { - int pct = static_castint(Available() * 100 / Capacity()); + int pct = Capacity() 0 ? static_castint(Available() * 100 / Capacity()) + : 0; PrintF( capacity: % V8_PTR_PREFIX d , waste: % V8_PTR_PREFIX -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Jeff Huang 發送給你一則邀請
Jeff Huang 發送給你一則邀請 此消息是經由輸入你的電子郵件地址之 Twitter 用戶發送。 接受邀請 https://twitter.com/i/08b7872d-710e-4bb3-a7b0-1d6223c92aba -- You can unsubscribe from receiving email notifications from Twitter at anytime. For general inquiries, please visit us at Twitter Support. 取消訂閱: https://twitter.com/i/o?t=1cn=aW52aXRlX3NlbGZfc2VydmU%3Diid=b23135a340b94a7182c391124a8838c9uid=0c=yATegVgYFPjLni7cH%2BDBPGF2kL2H47K1nid=244+26 需要幫助嗎? https://support.twitter.com -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Implement ES6 unicode escapes. (issue 706263002 by ma...@chromium.org)
https://codereview.chromium.org/706263002/diff/60001/src/scanner.cc File src/scanner.cc (right): https://codereview.chromium.org/706263002/diff/60001/src/scanner.cc#newcode908 src/scanner.cc:908: if (c0_ != 'u') return false; Will cause `var \x` to look like `var \u{0}x`, if I'm not mistaken https://codereview.chromium.org/706263002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 2935 in v8: Poor performance in Ember app Discourse
Comment #26 on issue 2935 by stefan.p...@gmail.com: Poor performance in Ember app Discourse https://code.google.com/p/v8/issues/detail?id=2935 Yup, thank to Ulan, I have been doing some experiments to remove some of these problem spots but while maintaining backwards compatibility. We will work on improving ember's side of things. Unfortunately a large amount of the excess map allocations are do to our malleable object model. I believe we can make improvements, but likely won't be able to remove these problematic areas entirely without breaking semver (and causing much community pain). I would like to push for reducing the flexibility of our object model anyways, good perf #'s will hopefully compensate future upgrade pains. As I make progress I'll report back here. :) -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: [turbofan] Weakening of types must handle ranges inside unions. (issue 712623002 by ja...@chromium.org)
Committed patchset #3 (id:40001) manually as 4c1f4b796d1c455fc6a023abe145a5e48c4b7b1f (presubmit successful). https://codereview.chromium.org/712623002/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Revert [turbofan] Weakening of types must weaken ranges inside unions. (issue 722943003 by ja...@chromium.org)
Reviewers: rossberg, Message: Committed patchset #1 (id:1) manually as c513297f9fdd1f4e6d19e0941fef55e22fb79d69 (tree was closed). Description: Revert [turbofan] Weakening of types must weaken ranges inside unions. This reverts commit 4c1f4b796d1c455fc6a023abe145a5e48c4b7b1f. TBR=rossb...@chromium.org Committed: https://chromium.googlesource.com/v8/v8/+/c513297f9fdd1f4e6d19e0941fef55e22fb79d69 Please review this at https://codereview.chromium.org/722943003/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+6, -78 lines): M src/compiler/typer.cc M src/types.h M src/types.cc M test/cctest/test-types.cc D test/mjsunit/regress/regress-weakening-multiplication.js Index: test/mjsunit/regress/regress-weakening-multiplication.js diff --git a/test/mjsunit/regress/regress-weakening-multiplication.js b/test/mjsunit/regress/regress-weakening-multiplication.js deleted file mode 100644 index dcf00114b7e46dfebb505eaeff57c484663c6fcb.. --- a/test/mjsunit/regress/regress-weakening-multiplication.js +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2014 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -function f() { - for (var j = 1; j 1; j *= -8) { - } - for (var i = 1; i 1; j += 2) { -j * -1; - } -} -f(); Index: src/compiler/typer.cc diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 20a39d94493ff1eed1463e1a142d7cd037c90655..97b43f4aefb989ac69e732a14c3fa77b954920a9 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -1119,9 +1119,10 @@ Bounds Typer::Visitor::TypeJSLoadNamed(Node* node) { // in the graph. In the current implementation, we are // increasing the limits to the closest power of two. Type* Typer::Visitor::Weaken(Type* current_type, Type* previous_type) { - Type::RangeType* previous = previous_type-GetRange(); - Type::RangeType* current = current_type-GetRange(); - if (previous != NULL current != NULL) { + if (current_type-IsRange() previous_type-IsRange()) { +Type::RangeType* previous = previous_type-AsRange(); +Type::RangeType* current = current_type-AsRange(); + double current_min = current-Min()-Number(); HandleObject new_min = current-Min(); @@ -1151,9 +1152,7 @@ Type* Typer::Visitor::Weaken(Type* current_type, Type* previous_type) { } } -return Type::Union(current_type, - Type::Range(new_min, new_max, typer_-zone()), - typer_-zone()); +return Type::Range(new_min, new_max, typer_-zone()); } return current_type; } Index: src/types.cc diff --git a/src/types.cc b/src/types.cc index b423beea0833e4be55865ff0e7d9fa463226afc4..d595b049d214a158cdb74056b884e1b26bd1e87e 100644 --- a/src/types.cc +++ b/src/types.cc @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include iomanip - #include src/types.h #include src/ostreams.h @@ -1019,12 +1017,8 @@ void TypeImplConfig::PrintTo(std::ostream os, PrintDimension dim) { } else if (this-IsConstant()) { os Constant( Brief(*this-AsConstant()-Value()) ); } else if (this-IsRange()) { - std::ostream::fmtflags saved_flags = os.setf(std::ios::fixed); - std::streamsize saved_precision = os.precision(0); os Range( this-AsRange()-Min()-Number() , this-AsRange()-Max()-Number() ); - os.flags(saved_flags); - os.precision(saved_precision); } else if (this-IsContext()) { os Context(; this-AsContext()-Outer()-PrintTo(os, dim); Index: src/types.h diff --git a/src/types.h b/src/types.h index aafaf07fb5ddc054378ea7a53be68545bb2006ef..1d506d0ca2eba3a2fa898ebde2530f20d5e421ee 100644 --- a/src/types.h +++ b/src/types.h @@ -464,11 +464,6 @@ class TypeImpl : public Config::Base { double Min(); double Max(); - // Extracts a range from the type. If the type is a range, it just - // returns it; if it is a union, it returns the range component. - // Note that it does not contain range for constants. - RangeType* GetRange(); - int NumClasses(); int NumConstants(); @@ -556,6 +551,7 @@ class TypeImpl : public Config::Base { static bool Contains(RangeType* lhs, RangeType* rhs); static bool Contains(RangeType* range, i::Object* val); + RangeType* GetRange(); static int UpdateRange( RangeHandle type, UnionHandle result, int size, Region* region); Index: test/cctest/test-types.cc diff --git a/test/cctest/test-types.cc b/test/cctest/test-types.cc index ad4660735928e384b7b986d9d5f8c2f5d8e3e2f9..e564c6c0808ab20f29d0fde37b12380d55343dcf 100644 --- a/test/cctest/test-types.cc +++ b/test/cctest/test-types.cc @@ -1831,48 +1831,6 @@ struct Tests : Rep { */ } - TypeHandle RangeToHandle(typename Type::RangeType* range) { -return T.Range(range-Min(),
[v8-dev] Re: Issue 3683 in v8: C-style for-let can't handle continue
Comment #16 on issue 3683 by rossb...@google.com: C-style for-let can't handle continue https://code.google.com/p/v8/issues/detail?id=3683 Ah, right. That was the reason of using a (different) flag to guard next in the current desugaring. I think it should be possible to combine that with the new desugaring I gave. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3683 in v8: C-style for-let can't handle continue
Comment #17 on issue 3683 by rossb...@google.com: C-style for-let can't handle continue https://code.google.com/p/v8/issues/detail?id=3683 That is: { let xs = inits let temp_xs = xs let first = true aux: for (;;) { let xs = temp_xs if (first) first = false else next let flag = true labels: for (; flag; temp_xs = xs, flag = false) if (cond) body else break aux if (flag) break } } Shouldn't that work? -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: Issue 3683 in v8: C-style for-let can't handle continue
Comment #18 on issue 3683 by ad...@chromium.org: C-style for-let can't handle continue https://code.google.com/p/v8/issues/detail?id=3683 Hmm, that does look OK. I was tripped up by needing to put the next outside the inner for loop. Thanks, will give it a shot. -- You received this message because this project is configured to send all issue notifications to this address. You may adjust your notification preferences at: https://code.google.com/hosting/settings -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] [turbofan] Avoid useless bit masking in typed lowering. (issue 718193003 by bmeu...@chromium.org)
Reviewers: jarin, Description: [turbofan] Avoid useless bit masking in typed lowering. There's no need to apply the 0x1f mask to right hand sides of shifts if the input is already in range [0,31]. TEST=cctest,unittests R=ja...@chromium.org Please review this at https://codereview.chromium.org/718193003/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+115, -5 lines): M src/compiler/js-typed-lowering.h M src/compiler/js-typed-lowering.cc M test/cctest/compiler/test-js-typed-lowering.cc M test/unittests/compiler/js-typed-lowering-unittest.cc -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: [turbofan] Avoid useless bit masking in typed lowering. (issue 718193003 by bmeu...@chromium.org)
PTAL https://codereview.chromium.org/718193003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: [turbofan] Avoid useless bit masking in typed lowering. (issue 718193003 by bmeu...@chromium.org)
lgtm https://codereview.chromium.org/718193003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Re: [turbofan] Avoid useless bit masking in typed lowering. (issue 718193003 by bmeu...@chromium.org)
Committed patchset #1 (id:1) manually as 7205f6ee9b795502b953c5545ebe2541980a9ebe (presubmit successful). https://codereview.chromium.org/718193003/ -- -- 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, send an email to v8-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[v8-dev] Reland [turbofan] Weakening of types must weaken ranges inside unions. (issue 723023002 by ja...@chromium.org)
Reviewers: rossberg, Message: Could you take a look, please? Unfortunately, GetRange is not really monotone, so I removed the test. The counterexample is Range(1, 2) = SignedSmall Union Range(1e+10, inf), but Range(1, 2) =/= Range(1e+10, inf). (This also shows that GetRange is really an ugly hack that we will have to address in a more principled way.) Description: Reland [turbofan] Weakening of types must weaken ranges inside unions. This relands commit 4c1f4b796d1c455fc6a023abe145a5e48c4b7b1f. R=rossb...@chromium.org Please review this at https://codereview.chromium.org/723023002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+55, -11 lines): M src/compiler/typer.cc M src/types.h M src/types.cc M test/cctest/test-types.cc A + test/mjsunit/regress/regress-weakening-multiplication.js Index: src/compiler/typer.cc diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 97b43f4aefb989ac69e732a14c3fa77b954920a9..20a39d94493ff1eed1463e1a142d7cd037c90655 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -1119,10 +1119,9 @@ Bounds Typer::Visitor::TypeJSLoadNamed(Node* node) { // in the graph. In the current implementation, we are // increasing the limits to the closest power of two. Type* Typer::Visitor::Weaken(Type* current_type, Type* previous_type) { - if (current_type-IsRange() previous_type-IsRange()) { -Type::RangeType* previous = previous_type-AsRange(); -Type::RangeType* current = current_type-AsRange(); - + Type::RangeType* previous = previous_type-GetRange(); + Type::RangeType* current = current_type-GetRange(); + if (previous != NULL current != NULL) { double current_min = current-Min()-Number(); HandleObject new_min = current-Min(); @@ -1152,7 +1151,9 @@ Type* Typer::Visitor::Weaken(Type* current_type, Type* previous_type) { } } -return Type::Range(new_min, new_max, typer_-zone()); +return Type::Union(current_type, + Type::Range(new_min, new_max, typer_-zone()), + typer_-zone()); } return current_type; } Index: src/types.cc diff --git a/src/types.cc b/src/types.cc index d595b049d214a158cdb74056b884e1b26bd1e87e..b423beea0833e4be55865ff0e7d9fa463226afc4 100644 --- a/src/types.cc +++ b/src/types.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include iomanip + #include src/types.h #include src/ostreams.h @@ -1017,8 +1019,12 @@ void TypeImplConfig::PrintTo(std::ostream os, PrintDimension dim) { } else if (this-IsConstant()) { os Constant( Brief(*this-AsConstant()-Value()) ); } else if (this-IsRange()) { + std::ostream::fmtflags saved_flags = os.setf(std::ios::fixed); + std::streamsize saved_precision = os.precision(0); os Range( this-AsRange()-Min()-Number() , this-AsRange()-Max()-Number() ); + os.flags(saved_flags); + os.precision(saved_precision); } else if (this-IsContext()) { os Context(; this-AsContext()-Outer()-PrintTo(os, dim); Index: src/types.h diff --git a/src/types.h b/src/types.h index 1d506d0ca2eba3a2fa898ebde2530f20d5e421ee..aafaf07fb5ddc054378ea7a53be68545bb2006ef 100644 --- a/src/types.h +++ b/src/types.h @@ -464,6 +464,11 @@ class TypeImpl : public Config::Base { double Min(); double Max(); + // Extracts a range from the type. If the type is a range, it just + // returns it; if it is a union, it returns the range component. + // Note that it does not contain range for constants. + RangeType* GetRange(); + int NumClasses(); int NumConstants(); @@ -551,7 +556,6 @@ class TypeImpl : public Config::Base { static bool Contains(RangeType* lhs, RangeType* rhs); static bool Contains(RangeType* range, i::Object* val); - RangeType* GetRange(); static int UpdateRange( RangeHandle type, UnionHandle result, int size, Region* region); Index: test/cctest/test-types.cc diff --git a/test/cctest/test-types.cc b/test/cctest/test-types.cc index e564c6c0808ab20f29d0fde37b12380d55343dcf..6e06890214d79a8fde4c828d06b0b6d4fe827113 100644 --- a/test/cctest/test-types.cc +++ b/test/cctest/test-types.cc @@ -1831,6 +1831,32 @@ struct Tests : Rep { */ } + void GetRange() { +// GetRange(Range(a, b)) = Range(a, b). +for (TypeIterator it1 = T.types.begin(); it1 != T.types.end(); ++it1) { + TypeHandle type1 = *it1; + if (type1-IsRange()) { +typename Type::RangeType* range = type1-GetRange(); +CHECK(type1-Min() == range-Min()-Number()); +CHECK(type1-Max() == range-Max()-Number()); + } +} + +// GetRange(Union(Constant(x), Range(min,max))) == Range(min, max). +for (TypeIterator it1 = T.types.begin(); it1 != T.types.end(); ++it1) { + for (TypeIterator it2 = T.types.begin(); it2 != T.types.end(); ++it2) { +TypeHandle type1 = *it1; +