From a stylistic point of view, this shouldn't just expose and call a
helper in
a completely unrelated class that happens to do something similar. Instead,
as
the condition becomes more complex, it should create its own helper
function to
clearly formulate the predicate, something like "static bool
IgnoreInputRepresentation(HPhi* phi, HValue* input)".
However, more importantly, I don't think we want this change in behavior.
For
the record, the testcase that regressed is equivalent to the following:
function test(array, initial_sum) {
var sum = initial_sum;
for (var i = 0; i < 100000000; i++) {
sum += array[sum];
}
return sum;
}
var a = [0];
for (var i = 0; i < 4; i++) {
test(a, 0);
}
So this is a very artificial micro-benchmark (and we don't want to waste our
time optimizing for artificial micro-benchmarks). And while I can confirm
that
for this particular situation, this CL helps, I can also give you an equally
artificial microbenchmark where it re-introduces the problem I've originally
fixed:
function repro(x, y) {
for (var i = 0; i < 100000; i++) {
if (typeof y != 'undefined') {
y = y | 0;
}
if (typeof y == 'undefined') {
y = 0;
}
}
return 1 - y;
}
for (var i = 0; i < 100; i++) {
repro(0);
}
This is a slight modification of the repro case from bug 3670, and in that
situation, current master deopts once (because of missing type feedback
after
the OSR'ed loop), whereas this CL deopts every single time and never learns.
Let's just leave the current behavior, at least as long as we don't hear
about
significant real-world regressions (which is unlikely at this point as
we've had
the change for almost a year).
Also, there's an easy workaround on the JS side if a developer cares about
this
particular pattern: adding "| 0" to the line
var sum = initial_sum | 0;
helps just as much as this CL.
https://codereview.chromium.org/1296423002/
--
--
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.
