Reviewers: Michael Achenbach,
Description:
Merged r19693, r19694 into 3.23 branch.
Fix HConstants with Smi-ranged HeapNumber values
Fix for failing asserts in HBoundsCheck code generation on x64: use proper
cmp
operation width instead of asserting that Integer32 values should be zero
extended. Similar to chromium:345820.
BUG=349878,349465
LOG=N
[email protected]
Please review this at https://codereview.chromium.org/198503004/
SVN Base: https://v8.googlecode.com/svn/branches/3.23
Affected files (+67, -28 lines):
M src/hydrogen-instructions.cc
M src/version.cc
M src/x64/lithium-codegen-x64.cc
A test/mjsunit/regress/regress-crbug-349465.js
A + test/mjsunit/regress/regress-crbug-349878.js
Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index
59a68140c18e9126cb9410c081f39b7d6fc1dcf0..86ce922ebb55384687f065b1a6125d32a00e2dda
100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -2572,7 +2572,11 @@ HConstant::HConstant(int32_t integer_value,
boolean_value_(integer_value != 0),
int32_value_(integer_value),
double_value_(FastI2D(integer_value)) {
- set_type(has_smi_value_ ? HType::Smi() : HType::TaggedNumber());
+ // It's possible to create a constant with a value in Smi-range but
stored
+ // in a (pre-existing) HeapNumber. See crbug.com/349878.
+ bool could_be_heapobject = r.IsTagged() && !object.handle().is_null();
+ bool is_smi = has_smi_value_ && !could_be_heapobject;
+ set_type(is_smi ? HType::Smi() : HType::TaggedNumber());
Initialize(r);
}
@@ -2592,7 +2596,11 @@ HConstant::HConstant(double double_value,
int32_value_(DoubleToInt32(double_value)),
double_value_(double_value) {
has_smi_value_ = has_int32_value_ && Smi::IsValid(int32_value_);
- set_type(has_smi_value_ ? HType::Smi() : HType::TaggedNumber());
+ // It's possible to create a constant with a value in Smi-range but
stored
+ // in a (pre-existing) HeapNumber. See crbug.com/349878.
+ bool could_be_heapobject = r.IsTagged() && !object.handle().is_null();
+ bool is_smi = has_smi_value_ && !could_be_heapobject;
+ set_type(is_smi ? HType::Smi() : HType::TaggedNumber());
Initialize(r);
}
Index: src/version.cc
diff --git a/src/version.cc b/src/version.cc
index
f8bb5a37cefac9e631d3b2b2571f54c982468d12..ac4596a4758c7830d1b44f6fc5070245e5a45f36
100644
--- a/src/version.cc
+++ b/src/version.cc
@@ -35,7 +35,7 @@
#define MAJOR_VERSION 3
#define MINOR_VERSION 23
#define BUILD_NUMBER 17
-#define PATCH_LEVEL 24
+#define PATCH_LEVEL 25
// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
#define IS_CANDIDATE_VERSION 0
Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index
26c249291c0e5a066b2f1626412c9eddd1daccf9..134d220fd2bfac29359c988361ae0cb5f963cd86
100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -4123,44 +4123,51 @@ void LCodeGen::ApplyCheckIf(Condition cc,
LBoundsCheck* check) {
void LCodeGen::DoBoundsCheck(LBoundsCheck* instr) {
- if (instr->hydrogen()->skip_check()) return;
+ HBoundsCheck* hinstr = instr->hydrogen();
+ if (hinstr->skip_check()) return;
+
+ Representation representation = hinstr->length()->representation();
+ ASSERT(representation.Equals(hinstr->index()->representation()));
+ ASSERT(representation.IsSmiOrInteger32());
if (instr->length()->IsRegister()) {
Register reg = ToRegister(instr->length());
- if (!instr->hydrogen()->length()->representation().IsSmi()) {
- __ AssertZeroExtended(reg);
- }
+
if (instr->index()->IsConstantOperand()) {
int32_t constant_index =
ToInteger32(LConstantOperand::cast(instr->index()));
- if (instr->hydrogen()->length()->representation().IsSmi()) {
+ if (representation.IsSmi()) {
__ Cmp(reg, Smi::FromInt(constant_index));
} else {
- __ cmpq(reg, Immediate(constant_index));
+ __ cmpl(reg, Immediate(constant_index));
}
} else {
Register reg2 = ToRegister(instr->index());
- if (!instr->hydrogen()->index()->representation().IsSmi()) {
- __ AssertZeroExtended(reg2);
+ if (representation.IsSmi()) {
+ __ cmpq(reg, reg2);
+ } else {
+ __ cmpl(reg, reg2);
}
- __ cmpq(reg, reg2);
}
} else {
Operand length = ToOperand(instr->length());
if (instr->index()->IsConstantOperand()) {
int32_t constant_index =
ToInteger32(LConstantOperand::cast(instr->index()));
- if (instr->hydrogen()->length()->representation().IsSmi()) {
+ if (representation.IsSmi()) {
__ Cmp(length, Smi::FromInt(constant_index));
} else {
- __ cmpq(length, Immediate(constant_index));
+ __ cmpl(length, Immediate(constant_index));
}
} else {
- __ cmpq(length, ToRegister(instr->index()));
+ if (representation.IsSmi()) {
+ __ cmpq(length, ToRegister(instr->index()));
+ } else {
+ __ cmpl(length, ToRegister(instr->index()));
+ }
}
}
- Condition condition =
- instr->hydrogen()->allow_equality() ? below : below_equal;
+ Condition condition = hinstr->allow_equality() ? below : below_equal;
ApplyCheckIf(condition, instr);
}
Index: test/mjsunit/regress/regress-crbug-349465.js
diff --git a/test/mjsunit/regress/regress-crbug-349465.js
b/test/mjsunit/regress/regress-crbug-349465.js
new file mode 100644
index
0000000000000000000000000000000000000000..335ea1e1a41337f5107161e7294038a662fc7d58
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-349465.js
@@ -0,0 +1,17 @@
+// 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.
+
+// Flags: --allow-natives-syntax --debug-code --use-gvn
+
+function f(a, base) {
+ a[base] = 1;
+ a[base + 4] = 2;
+ a[base] = 3;
+}
+var a1 = new Array(1024);
+var a2 = new Array(128);
+f(a1, 1);
+f(a2, -2);
+%OptimizeFunctionOnNextCall(f);
+f(a1, -2);
Index: test/mjsunit/regress/regress-crbug-349878.js
diff --git a/test/mjsunit/regress/regress-crbug-345715.js
b/test/mjsunit/regress/regress-crbug-349878.js
similarity index 55%
copy from test/mjsunit/regress/regress-crbug-345715.js
copy to test/mjsunit/regress/regress-crbug-349878.js
index
a3753417dfb6f8440ec36f883dc6ac1be4a6c8ce..5ed048ff5475df442cb3a3f0109db3c81780ddbc
100644
--- a/test/mjsunit/regress/regress-crbug-345715.js
+++ b/test/mjsunit/regress/regress-crbug-349878.js
@@ -4,23 +4,30 @@
// Flags: --allow-natives-syntax
-a = {y:1.5};
-a.y = 0;
-b = a.y;
-c = {y:{}};
+function f(a, b) {
+ a == b;
+}
+
+f({}, {});
-function f() {
- return 1;
+var a = { y: 1.5 };
+a.y = 777;
+var b = a.y;
+
+function h() {
+ var d = 1;
+ var e = 777;
+ while (d-- > 0) e++;
+ f(1, e);
}
+var global;
function g() {
- var e = {y: b};
- var d = {x:f()};
- var d = {x:f()};
- return [e, d];
+ global = b;
+ return h(b);
}
g();
g();
%OptimizeFunctionOnNextCall(g);
-assertEquals(1, g()[1].x);
+g();
--
--
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.
