Revision: 24434
Author: [email protected]
Date: Tue Oct 7 12:18:39 2014 UTC
Log: Fix bugs in lowering NumberToInt32.
The backward propagation was different in the lowering phase versus the
propagation phase, leading to some missing truncations.
[email protected]
BUG=
Review URL: https://codereview.chromium.org/638533002
https://code.google.com/p/v8/source/detail?r=24434
Modified:
/branches/bleeding_edge/src/compiler/simplified-lowering.cc
/branches/bleeding_edge/test/cctest/compiler/test-simplified-lowering.cc
=======================================
--- /branches/bleeding_edge/src/compiler/simplified-lowering.cc Wed Oct 1
14:03:02 2014 UTC
+++ /branches/bleeding_edge/src/compiler/simplified-lowering.cc Tue Oct 7
12:18:39 2014 UTC
@@ -502,44 +502,38 @@
}
case IrOpcode::kNumberToInt32: {
MachineTypeUnion use_rep = use & kRepMask;
- if (lower()) {
- MachineTypeUnion in = GetInfo(node->InputAt(0))->output;
- if ((in & kTypeMask) == kTypeInt32 || (in & kRepMask) ==
kRepWord32) {
- // If the input has type int32, or is already a word32, just
change
- // representation if necessary.
- VisitUnop(node, kTypeInt32 | use_rep, kTypeInt32 | use_rep);
- DeferReplacement(node, node->InputAt(0));
- } else {
- // Require the input in float64 format and perform truncation.
- // TODO(turbofan): avoid a truncation with a smi check.
- VisitUnop(node, kTypeInt32 | kRepFloat64, kTypeInt32 |
kRepWord32);
- node->set_op(lowering->machine()->TruncateFloat64ToInt32());
- }
+ Node* input = node->InputAt(0);
+ MachineTypeUnion in = GetInfo(input)->output;
+ if (NodeProperties::GetBounds(input).upper->Is(Type::Signed32()) ||
+ (in & kTypeMask) == kTypeInt32 || (in & kRepMask) ==
kRepWord32) {
+ // If the input has type int32, or is already a word32, just
change
+ // representation if necessary.
+ VisitUnop(node, kTypeInt32 | use_rep, kTypeInt32 | use_rep);
+ if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
- // Propagate a type to the input, but pass through
representation.
- VisitUnop(node, kTypeInt32, kTypeInt32 | use_rep);
+ // Require the input in float64 format and perform truncation.
+ // TODO(turbofan): avoid a truncation with a smi check.
+ VisitUnop(node, kTypeInt32 | kRepFloat64, kTypeInt32 |
kRepWord32);
+ if (lower())
+ node->set_op(lowering->machine()->TruncateFloat64ToInt32());
}
break;
}
case IrOpcode::kNumberToUint32: {
MachineTypeUnion use_rep = use & kRepMask;
- if (lower()) {
- MachineTypeUnion in = GetInfo(node->InputAt(0))->output;
- if ((in & kTypeMask) == kTypeUint32 ||
- (in & kRepMask) == kRepWord32) {
- // The input has type int32, just change representation.
- VisitUnop(node, kTypeUint32 | use_rep, kTypeUint32 | use_rep);
- DeferReplacement(node, node->InputAt(0));
- } else {
- // Require the input in float64 format to perform truncation.
- // TODO(turbofan): avoid the truncation with a smi check.
- VisitUnop(node, kTypeUint32 | kRepFloat64,
- kTypeUint32 | kRepWord32);
- node->set_op(lowering->machine()->TruncateFloat64ToInt32());
- }
+ Node* input = node->InputAt(0);
+ MachineTypeUnion in = GetInfo(input)->output;
+ if (NodeProperties::GetBounds(input).upper->Is(Type::Unsigned32())
||
+ (in & kTypeMask) == kTypeUint32) {
+ // If the input has type uint32, just change representation.
+ VisitUnop(node, kTypeUint32 | use_rep, kTypeUint32 | use_rep);
+ if (lower()) DeferReplacement(node, node->InputAt(0));
} else {
- // Propagate a type to the input, but pass through
representation.
- VisitUnop(node, kTypeUint32, kTypeUint32 | use_rep);
+ // Require the input in float64 format and perform truncation.
+ // TODO(turbofan): avoid a truncation with a smi check.
+ VisitUnop(node, kTypeUint32 | kRepFloat64, kTypeUint32 |
kRepWord32);
+ if (lower())
+ node->set_op(lowering->machine()->TruncateFloat64ToInt32());
}
break;
}
=======================================
---
/branches/bleeding_edge/test/cctest/compiler/test-simplified-lowering.cc
Wed Oct 1 15:47:40 2014 UTC
+++
/branches/bleeding_edge/test/cctest/compiler/test-simplified-lowering.cc
Tue Oct 7 12:18:39 2014 UTC
@@ -5,8 +5,10 @@
#include <limits>
#include "src/compiler/access-builder.h"
+#include "src/compiler/change-lowering.h"
#include "src/compiler/control-builders.h"
#include "src/compiler/generic-node-inl.h"
+#include "src/compiler/graph-reducer.h"
#include "src/compiler/graph-visualizer.h"
#include "src/compiler/node-properties-inl.h"
#include "src/compiler/pipeline.h"
@@ -50,6 +52,22 @@
this->End();
lowering.LowerAllNodes();
}
+
+ void LowerAllNodesAndLowerChanges() {
+ this->End();
+ typer.Run(jsgraph.graph(), MaybeHandle<Context>());
+ lowering.LowerAllNodes();
+
+ Zone* zone = this->zone();
+ CompilationInfo info(zone->isolate(), zone);
+ Linkage linkage(
+ &info, Linkage::GetSimplifiedCDescriptor(zone,
this->machine_sig_));
+ ChangeLowering lowering(&jsgraph, &linkage);
+ GraphReducer reducer(this->graph());
+ reducer.AddReducer(&lowering);
+ reducer.ReduceGraph();
+ Verifier::Run(this->graph());
+ }
Factory* factory() { return this->isolate()->factory(); }
Heap* heap() { return this->isolate()->heap(); }
@@ -66,6 +84,7 @@
FieldAccess load = {kUntaggedBase, 0, Handle<Name>(), Type::Number(),
kMachFloat64};
Node* loaded = t.LoadField(load, t.PointerConstant(&input));
+ NodeProperties::SetBounds(loaded, Bounds(Type::Number()));
Node* convert = t.NumberToInt32(loaded);
FieldAccess store = {kUntaggedBase, 0, Handle<Name>(), Type::Signed32(),
kMachInt32};
@@ -94,6 +113,7 @@
FieldAccess load = {kUntaggedBase, 0, Handle<Name>(), Type::Number(),
kMachFloat64};
Node* loaded = t.LoadField(load, t.PointerConstant(&input));
+ NodeProperties::SetBounds(loaded, Bounds(Type::Number()));
Node* convert = t.NumberToUint32(loaded);
FieldAccess store = {kUntaggedBase, 0, Handle<Name>(),
Type::Unsigned32(),
kMachUint32};
@@ -1004,6 +1024,8 @@
// NumberToInt32(x: kRepFloat64) used as kMachInt32
TestingGraph t(Type::Number());
Node* p0 = t.ExampleWithOutput(kMachFloat64);
+ // TODO(titzer): run the typer here, or attach machine type to param.
+ NodeProperties::SetBounds(p0, Bounds(Type::Number()));
Node* trunc = t.graph()->NewNode(t.simplified()->NumberToInt32(), p0);
Node* use = t.Use(trunc, kMachInt32);
t.Return(use);
@@ -1075,6 +1097,8 @@
// NumberToUint32(x: kRepFloat64) used as kMachUint32
TestingGraph t(Type::Number());
Node* p0 = t.ExampleWithOutput(kMachFloat64);
+ // TODO(titzer): run the typer here, or attach machine type to param.
+ NodeProperties::SetBounds(p0, Bounds(Type::Number()));
Node* trunc = t.graph()->NewNode(t.simplified()->NumberToUint32(), p0);
Node* use = t.Use(trunc, kMachUint32);
t.Return(use);
@@ -1526,3 +1550,60 @@
RepresentationOf(OpParameter<MachineType>(phi)));
}
}
+
+
+TEST(RunNumberDivide_minus_1_TruncatingToInt32) {
+ SimplifiedLoweringTester<Object*> t(kMachAnyTagged);
+ Node* num = t.NumberToInt32(t.Parameter(0));
+ Node* div = t.NumberDivide(num, t.jsgraph.Constant(-1));
+ Node* trunc = t.NumberToInt32(div);
+ t.Return(trunc);
+
+ if (Pipeline::SupportedTarget()) {
+ t.LowerAllNodesAndLowerChanges();
+ t.GenerateCode();
+
+ FOR_INT32_INPUTS(i) {
+ Handle<HeapNumber> num = t.factory()->NewHeapNumber(*i);
+ int32_t x = 0 - *i;
+ // TODO(titzer): make calls to NewHeapNumber work in cctests.
+ if (x <= Smi::kMinValue) continue;
+ if (x >= Smi::kMaxValue) continue;
+ Handle<HeapNumber> expected = t.factory()->NewHeapNumber(x);
+ Object* result = t.Call(*num);
+ CHECK(expected->SameValue(result));
+ }
+ }
+}
+
+
+TEST(RunNumberDivide_2_TruncatingToUint32) {
+ SimplifiedLoweringTester<Object*> t(kMachAnyTagged);
+ Node* num = t.NumberToUint32(t.Parameter(0));
+ Node* div = t.NumberDivide(num, t.jsgraph.Constant(2));
+ Node* trunc = t.NumberToUint32(div);
+ t.Return(trunc);
+
+ if (Pipeline::SupportedTarget()) {
+ t.LowerAllNodesAndLowerChanges();
+ {
+ FILE* dot_file = fopen("/tmp/test.dot", "w+");
+ OFStream dot_of(dot_file);
+ dot_of << AsDOT(*t.jsgraph.graph());
+ fclose(dot_file);
+ }
+ t.GenerateCode();
+
+ FOR_UINT32_INPUTS(i) {
+ Handle<HeapNumber> num =
+ t.factory()->NewHeapNumber(static_cast<double>(*i));
+ uint32_t x = *i / 2;
+ // TODO(titzer): make calls to NewHeapNumber work in cctests.
+ if (x >= static_cast<uint32_t>(Smi::kMaxValue)) continue;
+ Handle<HeapNumber> expected =
+ t.factory()->NewHeapNumber(static_cast<double>(x));
+ Object* result = t.Call(*num);
+ CHECK(expected->SameValue(result));
+ }
+ }
+}
--
--
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.
