[turbofan] Right hand side of shifts needs ToUint32.

Currently we lower shifts directly to machine operators, and add an
appropriate Word32And to implement the & 0x1F operation on the right
hand side required by the specification. However for Word32And we assume
Int32 in simplified lowering, which is basically changes the right hand
side bit interpretation for the shifts from Uint32 to Int32, which is
obviously wrong. So now we represent that explicitly by proper
simplified operators for the shifts, which are lowered to machine in
simplified lowering.

R=jarin@chromium.org

Review URL: https://codereview.chromium.org/1213803008

Cr-Commit-Position: refs/heads/master@{#29465}
This commit is contained in:
bmeurer 2015-07-03 04:41:54 -07:00 committed by Commit bot
parent 787de93f4c
commit 5f288c201c
16 changed files with 248 additions and 73 deletions

View File

@ -23,9 +23,6 @@ namespace compiler {
JSTypedLowering::JSTypedLowering(Editor* editor, JSGraph* jsgraph, Zone* zone)
: AdvancedReducer(editor), jsgraph_(jsgraph), simplified_(graph()->zone()) {
zero_range_ = Type::Range(0.0, 0.0, graph()->zone());
one_range_ = Type::Range(1.0, 1.0, graph()->zone());
zero_thirtyone_range_ = Type::Range(0.0, 31.0, graph()->zone());
for (size_t k = 0; k < arraysize(shifted_int32_ranges_); ++k) {
double min = kMinInt / (1 << k);
double max = kMaxInt / (1 << k);
@ -142,20 +139,6 @@ class JSBinopReduction final {
node_->ReplaceInput(1, ConvertToString(right()));
}
// Convert inputs for bitwise shift operation (ES5 spec 11.7).
void ConvertInputsForShift(Signedness left_signedness) {
node_->ReplaceInput(0, ConvertToUI32(ConvertPlainPrimitiveToNumber(left()),
left_signedness));
Node* rnum =
ConvertToUI32(ConvertPlainPrimitiveToNumber(right()), kUnsigned);
Type* rnum_type = NodeProperties::GetBounds(rnum).upper;
if (!rnum_type->Is(lowering_->zero_thirtyone_range_)) {
rnum = graph()->NewNode(machine()->Word32And(), rnum,
jsgraph()->Int32Constant(0x1F));
}
node_->ReplaceInput(1, rnum);
}
void SwapInputs() {
Node* l = left();
Node* r = right();
@ -488,14 +471,14 @@ Reduction JSTypedLowering::ReduceUI32Shift(Node* node,
JSBinopReduction r(this, node);
if (r.IsStrong()) {
if (r.BothInputsAre(Type::Number())) {
r.ConvertInputsForShift(left_signedness);
r.ConvertInputsToUI32(left_signedness, kUnsigned);
return r.ChangeToPureOperator(shift_op);
}
return NoChange();
}
Node* frame_state = NodeProperties::GetFrameStateInput(node, 1);
r.ConvertInputsToNumber(frame_state);
r.ConvertInputsForShift(left_signedness);
r.ConvertInputsToUI32(left_signedness, kUnsigned);
return r.ChangeToPureOperator(shift_op);
}
@ -1612,11 +1595,12 @@ Reduction JSTypedLowering::Reduce(Node* node) {
case IrOpcode::kJSBitwiseAnd:
return ReduceInt32Binop(node, machine()->Word32And());
case IrOpcode::kJSShiftLeft:
return ReduceUI32Shift(node, kSigned, machine()->Word32Shl());
return ReduceUI32Shift(node, kSigned, simplified()->NumberShiftLeft());
case IrOpcode::kJSShiftRight:
return ReduceUI32Shift(node, kSigned, machine()->Word32Sar());
return ReduceUI32Shift(node, kSigned, simplified()->NumberShiftRight());
case IrOpcode::kJSShiftRightLogical:
return ReduceUI32Shift(node, kUnsigned, machine()->Word32Shr());
return ReduceUI32Shift(node, kUnsigned,
simplified()->NumberShiftRightLogical());
case IrOpcode::kJSAdd:
return ReduceJSAdd(node);
case IrOpcode::kJSSubtract:

View File

@ -87,9 +87,6 @@ class JSTypedLowering final : public AdvancedReducer {
JSGraph* jsgraph_;
SimplifiedOperatorBuilder simplified_;
Type* zero_range_;
Type* one_range_;
Type* zero_thirtyone_range_;
Type* shifted_int32_ranges_[4];
};

View File

@ -171,6 +171,9 @@
V(NumberMultiply) \
V(NumberDivide) \
V(NumberModulus) \
V(NumberShiftLeft) \
V(NumberShiftRight) \
V(NumberShiftRightLogical) \
V(NumberToInt32) \
V(NumberToUint32) \
V(PlainPrimitiveToNumber) \

View File

@ -689,6 +689,21 @@ class RepresentationSelector {
if (lower()) node->set_op(Float64Op(node));
break;
}
case IrOpcode::kNumberShiftLeft: {
VisitBinop(node, kMachInt32, kMachUint32, kMachInt32);
if (lower()) lowering->DoShift(node, lowering->machine()->Word32Shl());
break;
}
case IrOpcode::kNumberShiftRight: {
VisitBinop(node, kMachInt32, kMachUint32, kMachInt32);
if (lower()) lowering->DoShift(node, lowering->machine()->Word32Sar());
break;
}
case IrOpcode::kNumberShiftRightLogical: {
VisitBinop(node, kMachUint32, kMachUint32, kMachUint32);
if (lower()) lowering->DoShift(node, lowering->machine()->Word32Shr());
break;
}
case IrOpcode::kNumberToInt32: {
MachineTypeUnion use_rep = use & kRepMask;
Node* input = node->InputAt(0);
@ -1118,6 +1133,14 @@ Node* SimplifiedLowering::IsTagged(Node* node) {
}
SimplifiedLowering::SimplifiedLowering(JSGraph* jsgraph, Zone* zone,
SourcePositionTable* source_positions)
: jsgraph_(jsgraph),
zone_(zone),
zero_thirtyone_range_(Type::Range(0, 31, zone)),
source_positions_(source_positions) {}
void SimplifiedLowering::LowerAllNodes() {
SimplifiedOperatorBuilder simplified(graph()->zone());
RepresentationChanger changer(jsgraph(), &simplified, jsgraph()->isolate());
@ -1576,6 +1599,17 @@ Node* SimplifiedLowering::Uint32Mod(Node* const node) {
}
void SimplifiedLowering::DoShift(Node* node, Operator const* op) {
node->set_op(op);
Node* const rhs = NodeProperties::GetValueInput(node, 1);
Type* const rhs_type = NodeProperties::GetBounds(rhs).upper;
if (!rhs_type->Is(zero_thirtyone_range_)) {
node->ReplaceInput(1, graph()->NewNode(machine()->Word32And(), rhs,
jsgraph()->Int32Constant(0x1f)));
}
}
void SimplifiedLowering::DoStringEqual(Node* node) {
node->set_op(machine()->WordEqual());
node->ReplaceInput(0, StringComparison(node, false));

View File

@ -21,8 +21,7 @@ class SourcePositionTable;
class SimplifiedLowering final {
public:
SimplifiedLowering(JSGraph* jsgraph, Zone* zone,
SourcePositionTable* source_positions)
: jsgraph_(jsgraph), zone_(zone), source_positions_(source_positions) {}
SourcePositionTable* source_positions);
~SimplifiedLowering() {}
void LowerAllNodes();
@ -38,6 +37,7 @@ class SimplifiedLowering final {
void DoStoreBuffer(Node* node);
void DoLoadElement(Node* node);
void DoStoreElement(Node* node);
void DoShift(Node* node, Operator const* op);
void DoStringEqual(Node* node);
void DoStringLessThan(Node* node);
void DoStringLessThanOrEqual(Node* node);
@ -45,6 +45,7 @@ class SimplifiedLowering final {
private:
JSGraph* const jsgraph_;
Zone* const zone_;
Type* const zero_thirtyone_range_;
// TODO(danno): SimplifiedLowering shouldn't know anything about the source
// positions table, but must for now since there currently is no other way to

View File

@ -157,32 +157,35 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
}
#define PURE_OP_LIST(V) \
V(BooleanNot, Operator::kNoProperties, 1) \
V(BooleanToNumber, Operator::kNoProperties, 1) \
V(NumberEqual, Operator::kCommutative, 2) \
V(NumberLessThan, Operator::kNoProperties, 2) \
V(NumberLessThanOrEqual, Operator::kNoProperties, 2) \
V(NumberAdd, Operator::kCommutative, 2) \
V(NumberSubtract, Operator::kNoProperties, 2) \
V(NumberMultiply, Operator::kCommutative, 2) \
V(NumberDivide, Operator::kNoProperties, 2) \
V(NumberModulus, Operator::kNoProperties, 2) \
V(NumberToInt32, Operator::kNoProperties, 1) \
V(NumberToUint32, Operator::kNoProperties, 1) \
V(PlainPrimitiveToNumber, Operator::kNoProperties, 1) \
V(StringEqual, Operator::kCommutative, 2) \
V(StringLessThan, Operator::kNoProperties, 2) \
V(StringLessThanOrEqual, Operator::kNoProperties, 2) \
V(ChangeTaggedToInt32, Operator::kNoProperties, 1) \
V(ChangeTaggedToUint32, Operator::kNoProperties, 1) \
V(ChangeTaggedToFloat64, Operator::kNoProperties, 1) \
V(ChangeInt32ToTagged, Operator::kNoProperties, 1) \
V(ChangeUint32ToTagged, Operator::kNoProperties, 1) \
V(ChangeFloat64ToTagged, Operator::kNoProperties, 1) \
V(ChangeBoolToBit, Operator::kNoProperties, 1) \
V(ChangeBitToBool, Operator::kNoProperties, 1) \
V(ObjectIsSmi, Operator::kNoProperties, 1) \
#define PURE_OP_LIST(V) \
V(BooleanNot, Operator::kNoProperties, 1) \
V(BooleanToNumber, Operator::kNoProperties, 1) \
V(NumberEqual, Operator::kCommutative, 2) \
V(NumberLessThan, Operator::kNoProperties, 2) \
V(NumberLessThanOrEqual, Operator::kNoProperties, 2) \
V(NumberAdd, Operator::kCommutative, 2) \
V(NumberSubtract, Operator::kNoProperties, 2) \
V(NumberMultiply, Operator::kCommutative, 2) \
V(NumberDivide, Operator::kNoProperties, 2) \
V(NumberModulus, Operator::kNoProperties, 2) \
V(NumberShiftLeft, Operator::kNoProperties, 2) \
V(NumberShiftRight, Operator::kNoProperties, 2) \
V(NumberShiftRightLogical, Operator::kNoProperties, 2) \
V(NumberToInt32, Operator::kNoProperties, 1) \
V(NumberToUint32, Operator::kNoProperties, 1) \
V(PlainPrimitiveToNumber, Operator::kNoProperties, 1) \
V(StringEqual, Operator::kCommutative, 2) \
V(StringLessThan, Operator::kNoProperties, 2) \
V(StringLessThanOrEqual, Operator::kNoProperties, 2) \
V(ChangeTaggedToInt32, Operator::kNoProperties, 1) \
V(ChangeTaggedToUint32, Operator::kNoProperties, 1) \
V(ChangeTaggedToFloat64, Operator::kNoProperties, 1) \
V(ChangeInt32ToTagged, Operator::kNoProperties, 1) \
V(ChangeUint32ToTagged, Operator::kNoProperties, 1) \
V(ChangeFloat64ToTagged, Operator::kNoProperties, 1) \
V(ChangeBoolToBit, Operator::kNoProperties, 1) \
V(ChangeBitToBool, Operator::kNoProperties, 1) \
V(ObjectIsSmi, Operator::kNoProperties, 1) \
V(ObjectIsNonNegativeSmi, Operator::kNoProperties, 1)

View File

@ -139,6 +139,9 @@ class SimplifiedOperatorBuilder final {
const Operator* NumberMultiply();
const Operator* NumberDivide();
const Operator* NumberModulus();
const Operator* NumberShiftLeft();
const Operator* NumberShiftRight();
const Operator* NumberShiftRightLogical();
const Operator* NumberToInt32();
const Operator* NumberToUint32();

View File

@ -1657,6 +1657,21 @@ Bounds Typer::Visitor::TypeNumberModulus(Node* node) {
}
Bounds Typer::Visitor::TypeNumberShiftLeft(Node* node) {
return Bounds(Type::None(zone()), Type::Signed32(zone()));
}
Bounds Typer::Visitor::TypeNumberShiftRight(Node* node) {
return Bounds(Type::None(zone()), Type::Signed32(zone()));
}
Bounds Typer::Visitor::TypeNumberShiftRightLogical(Node* node) {
return Bounds(Type::None(zone()), Type::Unsigned32(zone()));
}
Bounds Typer::Visitor::TypeNumberToInt32(Node* node) {
return TypeUnaryOp(node, NumberToInt32);
}

View File

@ -633,6 +633,19 @@ void Verifier::Visitor::Check(Node* node) {
// TODO(rossberg): activate once we retype after opcode changes.
// CheckUpperIs(node, Type::Number());
break;
case IrOpcode::kNumberShiftLeft:
case IrOpcode::kNumberShiftRight:
// (Signed32, Unsigned32) -> Signed32
CheckValueInputIs(node, 0, Type::Signed32());
CheckValueInputIs(node, 1, Type::Unsigned32());
CheckUpperIs(node, Type::Signed32());
break;
case IrOpcode::kNumberShiftRightLogical:
// (Unsigned32, Unsigned32) -> Unsigned32
CheckValueInputIs(node, 0, Type::Unsigned32());
CheckValueInputIs(node, 1, Type::Unsigned32());
CheckUpperIs(node, Type::Unsigned32());
break;
case IrOpcode::kNumberToInt32:
// Number -> Signed32
CheckValueInputIs(node, 0, Type::Number());

View File

@ -320,11 +320,11 @@ class JSBitwiseShiftTypedLoweringTester : public JSTypedLoweringTester {
: JSTypedLoweringTester(), language_mode_(language_mode) {
int i = 0;
set(i++, javascript.ShiftLeft(language_mode_), true);
set(i++, machine.Word32Shl(), false);
set(i++, simplified.NumberShiftLeft(), false);
set(i++, javascript.ShiftRight(language_mode_), true);
set(i++, machine.Word32Sar(), false);
set(i++, simplified.NumberShiftRight(), false);
set(i++, javascript.ShiftRightLogical(language_mode_), false);
set(i++, machine.Word32Shr(), false);
set(i++, simplified.NumberShiftRightLogical(), false);
}
static const int kNumberOps = 6;
const Operator* ops[kNumberOps];
@ -364,14 +364,7 @@ TEST(Int32BitwiseShifts) {
Node* r1 = r->InputAt(1);
CheckToI32(p0, r0, R.signedness[k]);
if (r1->opcode() == IrOpcode::kWord32And) {
R.CheckPureBinop(IrOpcode::kWord32And, r1);
CheckToI32(p1, r1->InputAt(0), R.signedness[k + 1]);
R.CheckInt32Constant(0x1F, r1->InputAt(1));
} else {
CheckToI32(p1, r1, R.signedness[k]);
}
CheckToI32(p1, r1, false);
}
}
}

View File

@ -0,0 +1,41 @@
// Copyright 2015 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
(function ShiftLeftWithDeoptUsage() {
function g() {}
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
g();
return 1 << tmp1;
}
%OptimizeFunctionOnNextCall(f);
assertEquals(512, f());
})();
(function ShiftLeftWithCallUsage() {
var f = (function() {
"use asm"
// This is not a valid asm.js, we use the "use asm" here to
// trigger Turbofan without deoptimization support.
function g(x) { return x; }
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
return g(1 << tmp1, tmp1);
}
return f;
})();
%OptimizeFunctionOnNextCall(f);
assertEquals(512, f());
})();

View File

@ -0,0 +1,41 @@
// Copyright 2015 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
(function ShiftRightLogicalWithDeoptUsage() {
function g() {}
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
g();
return 1 >>> tmp1;
}
%OptimizeFunctionOnNextCall(f);
assertEquals(0, f());
})();
(function ShiftRightLogicalWithCallUsage() {
var f = (function() {
"use asm"
// This is not a valid asm.js, we use the "use asm" here to
// trigger Turbofan without deoptimization support.
function g(x) { return x; }
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
return g(1 >>> tmp1, tmp1);
}
return f;
})();
%OptimizeFunctionOnNextCall(f);
assertEquals(0, f());
})();

View File

@ -0,0 +1,41 @@
// Copyright 2015 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
(function ShiftRightWithDeoptUsage() {
function g() {}
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
g();
return 1 >> tmp1;
}
%OptimizeFunctionOnNextCall(f);
assertEquals(0, f());
})();
(function ShiftRightWithCallUsage() {
var f = (function() {
"use asm"
// This is not a valid asm.js, we use the "use asm" here to
// trigger Turbofan without deoptimization support.
function g(x) { return x; }
function f() {
var tmp = 1264475713;
var tmp1 = tmp - (-913041544);
return g(1 >> tmp1, tmp1);
}
return f;
})();
%OptimizeFunctionOnNextCall(f);
assertEquals(0, f());
})();

View File

@ -467,7 +467,7 @@ TEST_F(JSTypedLoweringTest, JSShiftLeftWithSigned32AndConstant) {
context, EmptyFrameState(), EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Shl(lhs, IsNumberConstant(BitEq(rhs))));
IsNumberShiftLeft(lhs, IsNumberConstant(BitEq(rhs))));
}
}
}
@ -484,8 +484,7 @@ TEST_F(JSTypedLoweringTest, JSShiftLeftWithSigned32AndUnsigned32) {
javascript()->ShiftLeft(language_mode), lhs, rhs, context,
EmptyFrameState(), EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Shl(lhs, IsWord32And(rhs, IsInt32Constant(0x1f))));
EXPECT_THAT(r.replacement(), IsNumberShiftLeft(lhs, rhs));
}
}
@ -506,7 +505,7 @@ TEST_F(JSTypedLoweringTest, JSShiftRightWithSigned32AndConstant) {
context, EmptyFrameState(), EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Sar(lhs, IsNumberConstant(BitEq(rhs))));
IsNumberShiftRight(lhs, IsNumberConstant(BitEq(rhs))));
}
}
}
@ -523,8 +522,7 @@ TEST_F(JSTypedLoweringTest, JSShiftRightWithSigned32AndUnsigned32) {
javascript()->ShiftRight(language_mode), lhs, rhs, context,
EmptyFrameState(), EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Sar(lhs, IsWord32And(rhs, IsInt32Constant(0x1f))));
EXPECT_THAT(r.replacement(), IsNumberShiftRight(lhs, rhs));
}
}
@ -547,7 +545,7 @@ TEST_F(JSTypedLoweringTest,
EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Shr(lhs, IsNumberConstant(BitEq(rhs))));
IsNumberShiftRightLogical(lhs, IsNumberConstant(BitEq(rhs))));
}
}
}
@ -565,8 +563,7 @@ TEST_F(JSTypedLoweringTest,
javascript()->ShiftRightLogical(language_mode), lhs, rhs, context,
EmptyFrameState(), EmptyFrameState(), effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(),
IsWord32Shr(lhs, IsWord32And(rhs, IsInt32Constant(0x1f))));
EXPECT_THAT(r.replacement(), IsNumberShiftRightLogical(lhs, rhs));
}
}

View File

@ -1809,6 +1809,9 @@ IS_BINOP_MATCHER(NumberEqual)
IS_BINOP_MATCHER(NumberLessThan)
IS_BINOP_MATCHER(NumberSubtract)
IS_BINOP_MATCHER(NumberMultiply)
IS_BINOP_MATCHER(NumberShiftLeft)
IS_BINOP_MATCHER(NumberShiftRight)
IS_BINOP_MATCHER(NumberShiftRightLogical)
IS_BINOP_MATCHER(Word32And)
IS_BINOP_MATCHER(Word32Sar)
IS_BINOP_MATCHER(Word32Shl)

View File

@ -147,6 +147,12 @@ Matcher<Node*> IsNumberSubtract(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsNumberMultiply(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsNumberShiftLeft(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsNumberShiftRight(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsNumberShiftRightLogical(const Matcher<Node*>& lhs_matcher,
const Matcher<Node*>& rhs_matcher);
Matcher<Node*> IsAllocate(const Matcher<Node*>& size_matcher,
const Matcher<Node*>& effect_matcher,
const Matcher<Node*>& control_matcher);