 Chromium Code Reviews
 Chromium Code Reviews Issue 342035:
  Move the Location class into the AST Expression class as a member....  (Closed) 
  Base URL: http://v8.googlecode.com/svn/branches/bleeding_edge/
    
  
    Issue 342035:
  Move the Location class into the AST Expression class as a member....  (Closed) 
  Base URL: http://v8.googlecode.com/svn/branches/bleeding_edge/| Index: src/x64/fast-codegen-x64.cc | 
| =================================================================== | 
| --- src/x64/fast-codegen-x64.cc (revision 3175) | 
| +++ src/x64/fast-codegen-x64.cc (working copy) | 
| @@ -116,52 +116,41 @@ | 
| } | 
| -void FastCodeGenerator::Move(Location destination, Slot* source) { | 
| - switch (destination.type()) { | 
| - case Location::kUninitialized: | 
| +void FastCodeGenerator::Move(Expression::Context context, Slot* source) { | 
| + switch (context) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| __ push(Operand(rbp, SlotOffset(source))); | 
| break; | 
| } | 
| } | 
| -void FastCodeGenerator::Move(Location destination, Literal* expr) { | 
| - switch (destination.type()) { | 
| - case Location::kUninitialized: | 
| +void FastCodeGenerator::Move(Expression::Context context, Literal* expr) { | 
| + switch (context) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| __ Push(expr->handle()); | 
| break; | 
| } | 
| } | 
| -void FastCodeGenerator::Move(Slot* destination, Location source) { | 
| - switch (source.type()) { | 
| - case Location::kUninitialized: // Fall through. | 
| - case Location::kEffect: | 
| +void FastCodeGenerator::DropAndMove(Expression::Context context, | 
| + Register source) { | 
| + switch (context) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kValue: | 
| - __ pop(Operand(rbp, SlotOffset(destination))); | 
| - break; | 
| - } | 
| -} | 
| - | 
| - | 
| -void FastCodeGenerator::DropAndMove(Location destination, Register source) { | 
| - switch (destination.type()) { | 
| - case Location::kUninitialized: | 
| - UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| __ addq(rsp, Immediate(kPointerSize)); | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| __ movq(Operand(rsp, 0), source); | 
| break; | 
| } | 
| @@ -183,12 +172,9 @@ | 
| SetStatementPosition(stmt); | 
| Expression* expr = stmt->expression(); | 
| // Complete the statement based on the type of the subexpression. | 
| - if (expr->AsLiteral() != NULL) { | 
| - __ Move(rax, expr->AsLiteral()->handle()); | 
| - } else { | 
| - Visit(expr); | 
| - Move(rax, expr->location()); | 
| - } | 
| + Visit(expr); | 
| + ASSERT_EQ(Expression::kValue, expr->context()); | 
| + __ pop(rax); | 
| if (FLAG_trace) { | 
| __ push(rax); | 
| @@ -226,7 +212,7 @@ | 
| __ push(rsi); | 
| __ Push(boilerplate); | 
| __ CallRuntime(Runtime::kNewClosure, 2); | 
| - Move(expr->location(), rax); | 
| + Move(expr->context(), rax); | 
| } | 
| @@ -245,10 +231,10 @@ | 
| // A test rax instruction following the call is used by the IC to | 
| // indicate that the inobject property case was inlined. Ensure there | 
| // is no test rax instruction here. | 
| - DropAndMove(expr->location(), rax); | 
| + DropAndMove(expr->context(), rax); | 
| } else { | 
| Comment cmnt(masm_, "Stack slot"); | 
| - Move(expr->location(), rewrite->AsSlot()); | 
| + Move(expr->context(), rewrite->AsSlot()); | 
| } | 
| } | 
| @@ -276,7 +262,7 @@ | 
| __ CallRuntime(Runtime::kMaterializeRegExpLiteral, 4); | 
| // Label done: | 
| __ bind(&done); | 
| - Move(expr->location(), rax); | 
| + Move(expr->context(), rax); | 
| } | 
| @@ -329,7 +315,7 @@ | 
| case ObjectLiteral::Property::COMPUTED: | 
| if (key->handle()->IsSymbol()) { | 
| Visit(value); | 
| - ASSERT(value->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, value->context()); | 
| __ pop(rax); | 
| __ Move(rcx, key->handle()); | 
| Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); | 
| @@ -341,9 +327,9 @@ | 
| case ObjectLiteral::Property::PROTOTYPE: | 
| __ push(rax); | 
| Visit(key); | 
| - ASSERT(key->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, key->context()); | 
| Visit(value); | 
| - ASSERT(value->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, value->context()); | 
| __ CallRuntime(Runtime::kSetProperty, 3); | 
| __ movq(rax, Operand(rsp, 0)); // Restore result into rax. | 
| break; | 
| @@ -351,25 +337,25 @@ | 
| case ObjectLiteral::Property::GETTER: | 
| __ push(rax); | 
| Visit(key); | 
| - ASSERT(key->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, key->context()); | 
| __ Push(property->kind() == ObjectLiteral::Property::SETTER ? | 
| Smi::FromInt(1) : | 
| Smi::FromInt(0)); | 
| Visit(value); | 
| - ASSERT(value->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, value->context()); | 
| __ CallRuntime(Runtime::kDefineAccessor, 4); | 
| __ movq(rax, Operand(rsp, 0)); // Restore result into rax. | 
| break; | 
| default: UNREACHABLE(); | 
| } | 
| } | 
| - switch (expr->location().type()) { | 
| - case Location::kUninitialized: | 
| + switch (expr->context()) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| if (result_saved) __ addq(rsp, Immediate(kPointerSize)); | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| if (!result_saved) __ push(rax); | 
| break; | 
| } | 
| @@ -424,7 +410,7 @@ | 
| result_saved = true; | 
| } | 
| Visit(subexpr); | 
| - ASSERT(subexpr->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, subexpr->context()); | 
| // Store the subexpression value in the array's elements. | 
| __ pop(rax); // Subexpression value. | 
| @@ -437,13 +423,13 @@ | 
| __ RecordWrite(rbx, offset, rax, rcx); | 
| } | 
| - switch (expr->location().type()) { | 
| - case Location::kUninitialized: | 
| + switch (expr->context()) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| if (result_saved) __ addq(rsp, Immediate(kPointerSize)); | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| if (!result_saved) __ push(rax); | 
| break; | 
| } | 
| @@ -460,7 +446,6 @@ | 
| ASSERT(var->is_global() || var->slot() != NULL); | 
| Expression* rhs = expr->value(); | 
| - Location destination = expr->location(); | 
| 
William Hesse
2009/10/29 15:15:25
Why wasn't this giving an "unused" warning before?
 
Kevin Millikin (Chromium)
2009/10/29 16:36:10
Dunno.
 | 
| if (var->is_global()) { | 
| // Assignment to a global variable, use inline caching. Right-hand-side | 
| // value is passed in rax, variable name in rcx, and the global object | 
| @@ -470,7 +455,7 @@ | 
| if (rhs->AsLiteral() != NULL) { | 
| __ Move(rax, rhs->AsLiteral()->handle()); | 
| } else { | 
| - ASSERT(rhs->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, rhs->context()); | 
| Visit(rhs); | 
| __ pop(rax); | 
| } | 
| @@ -479,7 +464,7 @@ | 
| Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize)); | 
| __ Call(ic, RelocInfo::CODE_TARGET); | 
| // Overwrite the global object on the stack with the result if needed. | 
| - DropAndMove(expr->location(), rax); | 
| + DropAndMove(expr->context(), rax); | 
| } else { | 
| // Local or parameter assignment. | 
| @@ -489,18 +474,18 @@ | 
| // discarded result. Always perform the assignment. | 
| __ Move(kScratchRegister, rhs->AsLiteral()->handle()); | 
| __ movq(Operand(rbp, SlotOffset(var->slot())), kScratchRegister); | 
| - Move(expr->location(), kScratchRegister); | 
| + Move(expr->context(), kScratchRegister); | 
| } else { | 
| - ASSERT(rhs->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, rhs->context()); | 
| Visit(rhs); | 
| - switch (expr->location().type()) { | 
| - case Location::kUninitialized: | 
| + switch (expr->context()) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| // Case 'var = temp'. Discard right-hand-side temporary. | 
| - Move(var->slot(), rhs->location()); | 
| + __ pop(Operand(rbp, SlotOffset(var->slot()))); | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| // Case 'temp1 <- (var = temp0)'. Preserve right-hand-side | 
| // temporary on the stack. | 
| __ movq(kScratchRegister, Operand(rsp, 0)); | 
| @@ -544,16 +529,7 @@ | 
| // Drop key left on the stack by IC. | 
| __ addq(rsp, Immediate(kPointerSize)); | 
| } | 
| - switch (expr->location().type()) { | 
| - case Location::kUninitialized: | 
| - UNREACHABLE(); | 
| - case Location::kValue: | 
| - __ movq(Operand(rsp, 0), rax); | 
| - break; | 
| - case Location::kEffect: | 
| - __ addq(rsp, Immediate(kPointerSize)); | 
| - break; | 
| - } | 
| + DropAndMove(expr->context(), rax); | 
| } | 
| @@ -570,7 +546,7 @@ | 
| int arg_count = args->length(); | 
| for (int i = 0; i < arg_count; i++) { | 
| Visit(args->at(i)); | 
| - ASSERT(args->at(i)->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, args->at(i)->context()); | 
| } | 
| // Record source position for debugger | 
| SetSourcePosition(expr->position()); | 
| @@ -581,36 +557,36 @@ | 
| // Restore context register. | 
| __ movq(rsi, Operand(rbp, StandardFrameConstants::kContextOffset)); | 
| // Discard the function left on TOS. | 
| - DropAndMove(expr->location(), rax); | 
| + DropAndMove(expr->context(), rax); | 
| } | 
| -void FastCodeGenerator::VisitCallNew(CallNew* node) { | 
| +void FastCodeGenerator::VisitCallNew(CallNew* expr) { | 
| Comment cmnt(masm_, "[ CallNew"); | 
| // According to ECMA-262, section 11.2.2, page 44, the function | 
| // expression in new calls must be evaluated before the | 
| // arguments. | 
| // Push function on the stack. | 
| - Visit(node->expression()); | 
| - ASSERT(node->expression()->location().is_value()); | 
| + Visit(expr->expression()); | 
| + ASSERT_EQ(Expression::kValue, expr->expression()->context()); | 
| // If location is value, already on the stack, | 
| // Push global object (receiver). | 
| __ push(CodeGenerator::GlobalObject()); | 
| // Push the arguments ("left-to-right") on the stack. | 
| - ZoneList<Expression*>* args = node->arguments(); | 
| + ZoneList<Expression*>* args = expr->arguments(); | 
| int arg_count = args->length(); | 
| for (int i = 0; i < arg_count; i++) { | 
| Visit(args->at(i)); | 
| - ASSERT(args->at(i)->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, args->at(i)->context()); | 
| // If location is value, it is already on the stack, | 
| // so nothing to do here. | 
| } | 
| // Call the construct call builtin that handles allocation and | 
| // constructor invocation. | 
| - SetSourcePosition(node->position()); | 
| + SetSourcePosition(expr->position()); | 
| // Load function, arg_count into rdi and rax. | 
| __ Set(rax, arg_count); | 
| @@ -621,7 +597,7 @@ | 
| __ Call(construct_builtin, RelocInfo::CONSTRUCT_CALL); | 
| // Replace function on TOS with result in rax, or pop it. | 
| - DropAndMove(node->location(), rax); | 
| + DropAndMove(expr->context(), rax); | 
| } | 
| @@ -636,19 +612,19 @@ | 
| int arg_count = args->length(); | 
| for (int i = 0; i < arg_count; i++) { | 
| Visit(args->at(i)); | 
| - ASSERT(args->at(i)->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, args->at(i)->context()); | 
| } | 
| __ CallRuntime(function, arg_count); | 
| - Move(expr->location(), rax); | 
| + Move(expr->context(), rax); | 
| } | 
| void FastCodeGenerator::VisitBinaryOperation(BinaryOperation* expr) { | 
| switch (expr->op()) { | 
| case Token::COMMA: | 
| - ASSERT(expr->left()->location().is_effect()); | 
| - ASSERT_EQ(expr->right()->location().type(), expr->location().type()); | 
| + ASSERT_EQ(Expression::kEffect, expr->left()->context()); | 
| + ASSERT_EQ(expr->context(), expr->right()->context()); | 
| Visit(expr->left()); | 
| Visit(expr->right()); | 
| break; | 
| @@ -669,8 +645,8 @@ | 
| case Token::SHL: | 
| case Token::SHR: | 
| case Token::SAR: { | 
| - ASSERT(expr->left()->location().is_value()); | 
| - ASSERT(expr->right()->location().is_value()); | 
| + ASSERT_EQ(Expression::kValue, expr->left()->context()); | 
| + ASSERT_EQ(Expression::kValue, expr->right()->context()); | 
| Visit(expr->left()); | 
| Visit(expr->right()); | 
| @@ -678,7 +654,7 @@ | 
| NO_OVERWRITE, | 
| NO_GENERIC_BINARY_FLAGS); | 
| __ CallStub(&stub); | 
| - Move(expr->location(), rax); | 
| + Move(expr->context(), rax); | 
| break; | 
| } | 
| @@ -705,7 +681,7 @@ | 
| left_true = &eval_right; | 
| left_false = &done; | 
| } | 
| - Location destination = expr->location(); | 
| + Expression::Context context = expr->context(); | 
| Expression* left = expr->left(); | 
| Expression* right = expr->right(); | 
| @@ -717,19 +693,19 @@ | 
| // need it as the value of the whole expression. | 
| if (left->AsLiteral() != NULL) { | 
| __ Move(rax, left->AsLiteral()->handle()); | 
| - if (destination.is_value()) __ push(rax); | 
| + if (context == Expression::kValue) __ push(rax); | 
| } else { | 
| Visit(left); | 
| - ASSERT(left->location().is_value()); | 
| - switch (destination.type()) { | 
| - case Location::kUninitialized: | 
| + ASSERT_EQ(Expression::kValue, left->context()); | 
| + switch (context) { | 
| + case Expression::kUninitialized: | 
| UNREACHABLE(); | 
| - case Location::kEffect: | 
| + case Expression::kEffect: | 
| // Pop the left-hand value into rax because we will not need it as the | 
| // final result. | 
| __ pop(rax); | 
| break; | 
| - case Location::kValue: | 
| + case Expression::kValue: | 
| // Copy the left-hand value into rax because we may need it as the | 
| // final result. | 
| __ movq(rax, Operand(rsp, 0)); | 
| @@ -766,12 +742,12 @@ | 
| __ bind(&eval_right); | 
| // Discard the left-hand value if present on the stack. | 
| - if (destination.is_value()) { | 
| + if (context == Expression::kValue) { | 
| __ addq(rsp, Immediate(kPointerSize)); | 
| } | 
| // Save or discard the right-hand value as needed. | 
| Visit(right); | 
| - ASSERT_EQ(destination.type(), right->location().type()); | 
| + ASSERT_EQ(context, right->context()); | 
| __ bind(&done); | 
| } |