Revision: 8237
Author: [email protected]
Date: Thu Jun 9 05:27:28 2011
Log: Add boolean flag to HChange and LNumberUntagD to not convert
undefined to NaN.
This is needed so that HCompare, optimized for double inputs, works
correctly on undefined inputs.
BUG=v8:1434
TEST=mjsunit/bugs/bug-1434.js
Review URL: http://codereview.chromium.org/7044049
http://code.google.com/p/v8/source/detail?r=8237
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1434.js
Deleted:
/branches/bleeding_edge/test/mjsunit/bugs/bug-1434.js
Modified:
/branches/bleeding_edge/src/arm/lithium-arm.h
/branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
/branches/bleeding_edge/src/arm/lithium-codegen-arm.h
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.h
/branches/bleeding_edge/src/ia32/lithium-ia32.h
/branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
/branches/bleeding_edge/src/x64/lithium-codegen-x64.h
/branches/bleeding_edge/src/x64/lithium-x64.h
/branches/bleeding_edge/test/mjsunit/mjsunit.status
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1434.js Thu Jun 9
05:27:28 2011
@@ -0,0 +1,36 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+function compare(a, b) {
+ return a === b;
+}
+
+compare(1.5, 2.5);
+%OptimizeFunctionOnNextCall(compare);
+assertTrue(compare(undefined, undefined));
=======================================
--- /branches/bleeding_edge/test/mjsunit/bugs/bug-1434.js Wed Jun 8
00:45:37 2011
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright 2008 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-// * Redistributions of source code must retain the above copyright
-// notice, this list of conditions and the following disclaimer.
-// * Redistributions in binary form must reproduce the above
-// copyright notice, this list of conditions and the following
-// disclaimer in the documentation and/or other materials provided
-// with the distribution.
-// * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived
-// from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Flags: --allow-natives-syntax
-
-function compare(a, b) {
- return a === b;
-}
-
-compare(1.5, 2.5);
-%OptimizeFunctionOnNextCall(compare);
-assertTrue(compare(undefined, undefined));
=======================================
--- /branches/bleeding_edge/src/arm/lithium-arm.h Wed Jun 8 03:51:51 2011
+++ /branches/bleeding_edge/src/arm/lithium-arm.h Thu Jun 9 05:27:28 2011
@@ -1661,6 +1661,7 @@
}
DECLARE_CONCRETE_INSTRUCTION(NumberUntagD, "double-untag")
+ DECLARE_HYDROGEN_ACCESSOR(Change)
};
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Thu Jun 9
04:26:01 2011
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Thu Jun 9
05:27:28 2011
@@ -3836,6 +3836,7 @@
void LCodeGen::EmitNumberUntagD(Register input_reg,
DoubleRegister result_reg,
+ bool deoptimize_on_undefined,
LEnvironment* env) {
Register scratch = scratch0();
SwVfpRegister flt_scratch = s0;
@@ -3851,20 +3852,25 @@
__ ldr(scratch, FieldMemOperand(input_reg, HeapObject::kMapOffset));
__ LoadRoot(ip, Heap::kHeapNumberMapRootIndex);
__ cmp(scratch, Operand(ip));
- __ b(eq, &heap_number);
-
- __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
- __ cmp(input_reg, Operand(ip));
- DeoptimizeIf(ne, env);
-
- // Convert undefined to NaN.
- __ LoadRoot(ip, Heap::kNanValueRootIndex);
- __ sub(ip, ip, Operand(kHeapObjectTag));
- __ vldr(result_reg, ip, HeapNumber::kValueOffset);
- __ jmp(&done);
-
+ if (deoptimize_on_undefined) {
+ DeoptimizeIf(ne, env);
+ } else {
+ Label heap_number;
+ __ b(eq, &heap_number);
+
+ __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
+ __ cmp(input_reg, Operand(ip));
+ DeoptimizeIf(ne, env);
+
+ // Convert undefined to NaN.
+ __ LoadRoot(ip, Heap::kNanValueRootIndex);
+ __ sub(ip, ip, Operand(kHeapObjectTag));
+ __ vldr(result_reg, ip, HeapNumber::kValueOffset);
+ __ jmp(&done);
+
+ __ bind(&heap_number);
+ }
// Heap number to double register conversion.
- __ bind(&heap_number);
__ sub(ip, input_reg, Operand(kHeapObjectTag));
__ vldr(result_reg, ip, HeapNumber::kValueOffset);
__ jmp(&done);
@@ -3998,7 +4004,9 @@
Register input_reg = ToRegister(input);
DoubleRegister result_reg = ToDoubleRegister(result);
- EmitNumberUntagD(input_reg, result_reg, instr->environment());
+ EmitNumberUntagD(input_reg, result_reg,
+ instr->hydrogen()->deoptimize_on_undefined(),
+ instr->environment());
}
=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.h Tue May 24
07:01:36 2011
+++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.h Thu Jun 9
05:27:28 2011
@@ -266,6 +266,7 @@
void EmitCmpI(LOperand* left, LOperand* right);
void EmitNumberUntagD(Register input,
DoubleRegister result,
+ bool deoptimize_on_undefined,
LEnvironment* env);
// Emits optimized code for typeof x == "y". Modifies input register.
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Thu Jun 9
04:15:03 2011
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Thu Jun 9
05:27:28 2011
@@ -1240,6 +1240,10 @@
if (r.IsTagged()) {
SetAllSideEffects();
ClearFlag(kUseGVN);
+ } else if (r.IsDouble()) {
+ SetFlag(kDeoptimizeOnUndefined);
+ ClearAllSideEffects();
+ SetFlag(kUseGVN);
} else {
ClearAllSideEffects();
SetFlag(kUseGVN);
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Thu Jun 9 04:15:03
2011
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Thu Jun 9 05:27:28
2011
@@ -487,6 +487,7 @@
kCanOverflow,
kBailoutOnMinusZero,
kCanBeDivByZero,
+ kDeoptimizeOnUndefined,
kIsArguments,
kTruncatingToInt32,
kLastFlag = kTruncatingToInt32
@@ -1067,8 +1068,11 @@
HChange(HValue* value,
Representation from,
Representation to,
- bool is_truncating)
- : HUnaryOperation(value), from_(from) {
+ bool is_truncating,
+ bool deoptimize_on_undefined)
+ : HUnaryOperation(value),
+ from_(from),
+ deoptimize_on_undefined_(deoptimize_on_undefined) {
ASSERT(!from.IsNone() && !to.IsNone());
ASSERT(!from.Equals(to));
set_representation(to);
@@ -1084,6 +1088,7 @@
Representation from() const { return from_; }
Representation to() const { return representation(); }
+ bool deoptimize_on_undefined() const { return deoptimize_on_undefined_; }
virtual Representation RequiredInputRepresentation(int index) const {
return from_;
}
@@ -1097,11 +1102,13 @@
if (!other->IsChange()) return false;
HChange* change = HChange::cast(other);
return value() == change->value()
- && to().Equals(change->to());
+ && to().Equals(change->to())
+ && deoptimize_on_undefined() == change->deoptimize_on_undefined();
}
private:
Representation from_;
+ bool deoptimize_on_undefined_;
};
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Thu Jun 9 04:26:01 2011
+++ /branches/bleeding_edge/src/hydrogen.cc Thu Jun 9 05:27:28 2011
@@ -1850,6 +1850,8 @@
// change instructions for them.
HInstruction* new_value = NULL;
bool is_truncating = use_value->CheckFlag(HValue::kTruncatingToInt32);
+ bool deoptimize_on_undefined =
+ use_value->CheckFlag(HValue::kDeoptimizeOnUndefined);
if (value->IsConstant()) {
HConstant* constant = HConstant::cast(value);
// Try to create a new copy of the constant with the new
representation.
@@ -1859,8 +1861,8 @@
}
if (new_value == NULL) {
- new_value =
- new(zone()) HChange(value, value->representation(), to,
is_truncating);
+ new_value = new(zone()) HChange(value, value->representation(), to,
+ is_truncating,
deoptimize_on_undefined);
}
new_value->InsertBefore(next);
@@ -1940,6 +1942,40 @@
}
}
}
+
+
+void HGraph::RecursivelyMarkPhiDeoptimizeOnUndefined(HPhi* phi) {
+ if (phi->CheckFlag(HValue::kDeoptimizeOnUndefined)) return;
+ phi->SetFlag(HValue::kDeoptimizeOnUndefined);
+ for (int i = 0; i < phi->OperandCount(); ++i) {
+ HValue* input = phi->OperandAt(i);
+ if (input->IsPhi()) {
+ RecursivelyMarkPhiDeoptimizeOnUndefined(HPhi::cast(input));
+ }
+ }
+}
+
+
+void HGraph::MarkDeoptimizeOnUndefined() {
+ HPhase phase("MarkDeoptimizeOnUndefined", this);
+ // Compute DeoptimizeOnUndefined flag for phis.
+ // Any phi that can reach a use with DeoptimizeOnUndefined set must
+ // have DeoptimizeOnUndefined set. Currently only HCompare, with
+ // double input representation, has this flag set.
+ // The flag is used by HChange tagged->double, which must deoptimize
+ // if one of its uses has this flag set.
+ for (int i = 0; i < phi_list()->length(); i++) {
+ HPhi* phi = phi_list()->at(i);
+ if (phi->representation().IsDouble()) {
+ for (HUseIterator it(phi->uses()); !it.Done(); it.Advance()) {
+ if (it.value()->CheckFlag(HValue::kDeoptimizeOnUndefined)) {
+ RecursivelyMarkPhiDeoptimizeOnUndefined(phi);
+ break;
+ }
+ }
+ }
+ }
+}
void HGraph::ComputeMinusZeroChecks() {
@@ -2248,6 +2284,7 @@
graph()->InitializeInferredTypes();
graph()->Canonicalize();
+ graph()->MarkDeoptimizeOnUndefined();
graph()->InsertRepresentationChanges();
graph()->ComputeMinusZeroChecks();
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Thu Jun 9 04:15:03 2011
+++ /branches/bleeding_edge/src/hydrogen.h Thu Jun 9 05:27:28 2011
@@ -216,6 +216,7 @@
void InitializeInferredTypes();
void InsertTypeConversions();
void InsertRepresentationChanges();
+ void MarkDeoptimizeOnUndefined();
void ComputeMinusZeroChecks();
bool ProcessArgumentsObject();
void EliminateRedundantPhis();
@@ -280,6 +281,7 @@
void InsertTypeConversions(HInstruction* instr);
void PropagateMinusZeroChecks(HValue* value, BitVector* visited);
+ void RecursivelyMarkPhiDeoptimizeOnUndefined(HPhi* phi);
void InsertRepresentationChangeForUse(HValue* value,
HValue* use_value,
int use_index,
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Thu Jun 9
04:26:01 2011
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Thu Jun 9
05:27:28 2011
@@ -3617,8 +3617,9 @@
void LCodeGen::EmitNumberUntagD(Register input_reg,
XMMRegister result_reg,
+ bool deoptimize_on_undefined,
LEnvironment* env) {
- Label load_smi, heap_number, done;
+ Label load_smi, done;
// Smi check.
__ test(input_reg, Immediate(kSmiTagMask));
@@ -3627,18 +3628,23 @@
// Heap number map check.
__ cmp(FieldOperand(input_reg, HeapObject::kMapOffset),
factory()->heap_number_map());
- __ j(equal, &heap_number, Label::kNear);
-
- __ cmp(input_reg, factory()->undefined_value());
- DeoptimizeIf(not_equal, env);
-
- // Convert undefined to NaN.
- ExternalReference nan = ExternalReference::address_of_nan();
- __ movdbl(result_reg, Operand::StaticVariable(nan));
- __ jmp(&done, Label::kNear);
-
+ if (deoptimize_on_undefined) {
+ DeoptimizeIf(not_equal, env);
+ } else {
+ Label heap_number;
+ __ j(equal, &heap_number, Label::kNear);
+
+ __ cmp(input_reg, factory()->undefined_value());
+ DeoptimizeIf(not_equal, env);
+
+ // Convert undefined to NaN.
+ ExternalReference nan = ExternalReference::address_of_nan();
+ __ movdbl(result_reg, Operand::StaticVariable(nan));
+ __ jmp(&done, Label::kNear);
+
+ __ bind(&heap_number);
+ }
// Heap number to XMM conversion.
- __ bind(&heap_number);
__ movdbl(result_reg, FieldOperand(input_reg, HeapNumber::kValueOffset));
__ jmp(&done, Label::kNear);
@@ -3770,7 +3776,9 @@
Register input_reg = ToRegister(input);
XMMRegister result_reg = ToDoubleRegister(result);
- EmitNumberUntagD(input_reg, result_reg, instr->environment());
+ EmitNumberUntagD(input_reg, result_reg,
+ instr->hydrogen()->deoptimize_on_undefined(),
+ instr->environment());
}
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.h Tue May 24
07:01:36 2011
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.h Thu Jun 9
05:27:28 2011
@@ -261,7 +261,10 @@
void EmitGoto(int block, LDeferredCode* deferred_stack_check = NULL);
void EmitBranch(int left_block, int right_block, Condition cc);
void EmitCmpI(LOperand* left, LOperand* right);
- void EmitNumberUntagD(Register input, XMMRegister result, LEnvironment*
env);
+ void EmitNumberUntagD(Register input,
+ XMMRegister result,
+ bool deoptimize_on_undefined,
+ LEnvironment* env);
// Emits optimized code for typeof x == "y". Modifies input register.
// Returns the condition on which a final split to
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.h Wed Jun 8 03:51:51 2011
+++ /branches/bleeding_edge/src/ia32/lithium-ia32.h Thu Jun 9 05:27:28 2011
@@ -1705,6 +1705,7 @@
}
DECLARE_CONCRETE_INSTRUCTION(NumberUntagD, "double-untag")
+ DECLARE_HYDROGEN_ACCESSOR(Change);
};
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Thu Jun 9
04:26:01 2011
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Thu Jun 9
05:27:28 2011
@@ -3559,8 +3559,9 @@
void LCodeGen::EmitNumberUntagD(Register input_reg,
XMMRegister result_reg,
+ bool deoptimize_on_undefined,
LEnvironment* env) {
- Label load_smi, heap_number, done;
+ Label load_smi, done;
// Smi check.
__ JumpIfSmi(input_reg, &load_smi, Label::kNear);
@@ -3568,18 +3569,23 @@
// Heap number map check.
__ CompareRoot(FieldOperand(input_reg, HeapObject::kMapOffset),
Heap::kHeapNumberMapRootIndex);
- __ j(equal, &heap_number, Label::kNear);
-
- __ CompareRoot(input_reg, Heap::kUndefinedValueRootIndex);
- DeoptimizeIf(not_equal, env);
-
- // Convert undefined to NaN. Compute NaN as 0/0.
- __ xorps(result_reg, result_reg);
- __ divsd(result_reg, result_reg);
- __ jmp(&done, Label::kNear);
-
+ if (deoptimize_on_undefined) {
+ DeoptimizeIf(not_equal, env);
+ } else {
+ Label heap_number;
+ __ j(equal, &heap_number, Label::kNear);
+
+ __ CompareRoot(input_reg, Heap::kUndefinedValueRootIndex);
+ DeoptimizeIf(not_equal, env);
+
+ // Convert undefined to NaN. Compute NaN as 0/0.
+ __ xorps(result_reg, result_reg);
+ __ divsd(result_reg, result_reg);
+ __ jmp(&done, Label::kNear);
+
+ __ bind(&heap_number);
+ }
// Heap number to XMM conversion.
- __ bind(&heap_number);
__ movsd(result_reg, FieldOperand(input_reg, HeapNumber::kValueOffset));
__ jmp(&done, Label::kNear);
@@ -3670,7 +3676,9 @@
Register input_reg = ToRegister(input);
XMMRegister result_reg = ToDoubleRegister(result);
- EmitNumberUntagD(input_reg, result_reg, instr->environment());
+ EmitNumberUntagD(input_reg, result_reg,
+ instr->hydrogen()->deoptimize_on_undefined(),
+ instr->environment());
}
=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.h Tue May 24
07:01:36 2011
+++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.h Thu Jun 9
05:27:28 2011
@@ -249,7 +249,10 @@
void EmitGoto(int block, LDeferredCode* deferred_stack_check = NULL);
void EmitBranch(int left_block, int right_block, Condition cc);
void EmitCmpI(LOperand* left, LOperand* right);
- void EmitNumberUntagD(Register input, XMMRegister result, LEnvironment*
env);
+ void EmitNumberUntagD(Register input,
+ XMMRegister result,
+ bool deoptimize_on_undefined,
+ LEnvironment* env);
// Emits optimized code for typeof x == "y". Modifies input register.
// Returns the condition on which a final split to
=======================================
--- /branches/bleeding_edge/src/x64/lithium-x64.h Wed Jun 8 03:51:51 2011
+++ /branches/bleeding_edge/src/x64/lithium-x64.h Thu Jun 9 05:27:28 2011
@@ -1651,6 +1651,7 @@
}
DECLARE_CONCRETE_INSTRUCTION(NumberUntagD, "double-untag")
+ DECLARE_HYDROGEN_ACCESSOR(Change);
};
=======================================
--- /branches/bleeding_edge/test/mjsunit/mjsunit.status Wed Jun 8 00:45:37
2011
+++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Thu Jun 9 05:27:28
2011
@@ -30,9 +30,6 @@
# All tests in the bug directory are expected to fail.
bugs: FAIL
-# Reliably fails when crankshaft is active, passes with --nocrankshaft
-bugs/bug-1434: FAIL || PASS
-
##############################################################################
# Too slow in debug mode with --stress-opt
compiler/regress-stacktrace-methods: PASS, SKIP if $mode == debug
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
