LGTM with comments.
https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-feedback.cc
File src/compiler/js-type-feedback.cc (right):
https://codereview.chromium.org/1063513003/diff/20001/src/compiler/js-type-feedback.cc#newcode232
src/compiler/js-type-feedback.cc:232:
String::Flatten(Handle<String>::cast(constant_value));
rant: We need to flatten strings here? SRSLY? OMG!
https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):
https://codereview.chromium.org/1063513003/diff/20001/src/compiler/pipeline.cc#newcode523
src/compiler/pipeline.cc:523: if (data->info()->has_global_object()) {
This is only valid if we don't share TurboFan code accross native
contexts. We are starting to add more and more assumptions like this. I
think it's time to introduce some sort of "sharing mode" that let's the
pipeline (and the different reducers) know what level of sharing is
expected. WDYT?
Can we leave TODO in that regard here and introduce the "sharing mode"
in a follow-up CL?
https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-delete.js
File test/mjsunit/global-delete.js (right):
https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-delete.js#newcode3
test/mjsunit/global-delete.js:3: // found in the LICENSE file.
suggestion: Should we move this file into the "compiler" supdirectory of
mjsunit? Because it seems to be targeting optimized code specifically.
https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-var-delete.js
File test/mjsunit/global-var-delete.js (right):
https://codereview.chromium.org/1063513003/diff/20001/test/mjsunit/global-var-delete.js#newcode3
test/mjsunit/global-var-delete.js:3: // found in the LICENSE file.
suggestion: Should we move this file into the "compiler" supdirectory of
mjsunit? Because it seems to be targeting optimized code specifically.
https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler/js-type-feedback-unittest.cc
File test/unittests/compiler/js-type-feedback-unittest.cc (right):
https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler/js-type-feedback-unittest.cc#newcode71
test/unittests/compiler/js-type-feedback-unittest.cc:71: USE(result);
nit: result.Check() or at least result.Assert() here.
https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler/js-type-feedback-unittest.cc#newcode102
test/unittests/compiler/js-type-feedback-unittest.cc:102: for (int i =
FLAG_turbo_deoptimization = 0; i < 2; \
nit: Assigning integer to boolean looks fishy.
https://codereview.chromium.org/1063513003/diff/20001/test/unittests/compiler/js-type-feedback-unittest.cc#newcode103
test/unittests/compiler/js-type-feedback-unittest.cc:103:
FLAG_turbo_deoptimization = ++i)
nit: Likewise.
https://codereview.chromium.org/1063513003/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
