From 846688f825aa84b1098f8fb365ddb24cddbebbf4 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Mon, 19 Oct 2009 10:36:42 +0000 Subject: [PATCH] Recognize in the fast-mode code generator when a subexpression is a constant known at compile time. Do not ever use the stack to materialize (non-function-argument) constants. Currently, constants are only the non-materialized, non-function literals in the AST. It is a known issue that there is no test coverage for the cases of assigning a non-literal to a variable and returning a literal. Those code paths are unreachable and tests will be added when they become reachable. For the code '.result = true', we had previously on ia32: 27 push 0xf5c28161 ;; object: 0xf5c28161 32 pop [ebp+0xf4] Now: 27 mov eax,0xf5c26161 ;; object: 0xf5c26161 32 mov [ebp+0xf4],eax ======== We had previously on x64: 25 movq r10,0x7fb8c2f78199 ;; object: 0x7fb8c2f78199 35 push r10 37 pop [rbp-0x18] Now: 25 movq r10,0x7fb131386199 ;; object: 0x7fb131386199 35 movq [rbp-0x18],r10 The generated code for ARM did not include the extra memory traffic. It was already eliminated by the ARM assembler's push/pop elimination. Review URL: http://codereview.chromium.org/300003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3088 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/fast-codegen-arm.cc | 64 ++++++++++++++++++++++++----------- src/compiler.cc | 1 + src/fast-codegen.cc | 5 +++ src/ia32/fast-codegen-ia32.cc | 62 +++++++++++++++++++++++---------- src/location.h | 1 + src/x64/fast-codegen-x64.cc | 63 +++++++++++++++++++++++----------- 6 files changed, 139 insertions(+), 57 deletions(-) diff --git a/src/arm/fast-codegen-arm.cc b/src/arm/fast-codegen-arm.cc index d2e620cfdf..6b485474da 100644 --- a/src/arm/fast-codegen-arm.cc +++ b/src/arm/fast-codegen-arm.cc @@ -114,8 +114,19 @@ void FastCodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) { void FastCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) { Comment cmnt(masm_, "[ ReturnStatement"); SetStatementPosition(stmt); - Visit(stmt->expression()); - __ pop(r0); + Expression* expr = stmt->expression(); + Visit(expr); + + // Complete the statement based on the location of the subexpression. + Location source = expr->location(); + ASSERT(!source.is_nowhere()); + if (source.is_temporary()) { + __ pop(r0); + } else { + ASSERT(source.is_constant()); + ASSERT(expr->AsLiteral() != NULL); + __ mov(r0, Operand(expr->AsLiteral()->handle())); + } __ RecordJSReturn(); __ mov(sp, fp); __ ldm(ia_w, sp, fp.bit() | lr.bit()); @@ -143,33 +154,46 @@ void FastCodeGenerator::VisitVariableProxy(VariableProxy* expr) { } -void FastCodeGenerator::VisitLiteral(Literal* expr) { - Comment cmnt(masm_, "[ Literal"); - if (expr->location().is_temporary()) { - __ mov(ip, Operand(expr->handle())); - __ push(ip); - } else { - ASSERT(expr->location().is_nowhere()); - } -} - - void FastCodeGenerator::VisitAssignment(Assignment* expr) { Comment cmnt(masm_, "[ Assignment"); ASSERT(expr->op() == Token::ASSIGN || expr->op() == Token::INIT_VAR); + Expression* rhs = expr->value(); + Visit(rhs); - Visit(expr->value()); - + // Left-hand side is always a (parameter or local) slot. Variable* var = expr->target()->AsVariableProxy()->AsVariable(); ASSERT(var != NULL && var->slot() != NULL); - if (expr->location().is_temporary()) { - __ ldr(ip, MemOperand(sp)); + // Complete the assignment based on the location of the right-hand-side + // value and the desired location of the assignment value. + Location destination = expr->location(); + Location source = rhs->location(); + ASSERT(!destination.is_constant()); + ASSERT(!source.is_nowhere()); + + if (source.is_temporary()) { + if (destination.is_temporary()) { + // Case 'temp1 <- (var = temp0)'. Preserve right-hand-side temporary + // on the stack. + __ ldr(ip, MemOperand(sp)); + } else { + ASSERT(destination.is_nowhere()); + // Case 'var = temp'. Discard right-hand-side temporary. + __ pop(ip); + } + __ str(ip, MemOperand(fp, SlotOffset(var->slot()))); } else { - ASSERT(expr->location().is_nowhere()); - __ pop(ip); + ASSERT(source.is_constant()); + ASSERT(rhs->AsLiteral() != NULL); + // Two cases: 'temp <- (var = constant)', or 'var = constant' with a + // discarded result. Always perform the assignment. + __ mov(ip, Operand(rhs->AsLiteral()->handle())); + __ str(ip, MemOperand(fp, SlotOffset(var->slot()))); + if (destination.is_temporary()) { + // Case 'temp <- (var = constant)'. Save result. + __ push(ip); + } } - __ str(ip, MemOperand(fp, SlotOffset(var->slot()))); } diff --git a/src/compiler.cc b/src/compiler.cc index 2e55683b2b..0bb0cc730d 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -614,6 +614,7 @@ void CodeGenSelector::VisitVariableProxy(VariableProxy* expr) { void CodeGenSelector::VisitLiteral(Literal* expr) { // All literals are supported. + expr->set_location(Location::Constant()); } diff --git a/src/fast-codegen.cc b/src/fast-codegen.cc index 4ec6a524f2..e67a149fc2 100644 --- a/src/fast-codegen.cc +++ b/src/fast-codegen.cc @@ -196,6 +196,11 @@ void FastCodeGenerator::VisitSlot(Slot* expr) { } +void FastCodeGenerator::VisitLiteral(Literal* expr) { + // No code is emitted (here) for simple literals. +} + + void FastCodeGenerator::VisitRegExpLiteral(RegExpLiteral* expr) { UNREACHABLE(); } diff --git a/src/ia32/fast-codegen-ia32.cc b/src/ia32/fast-codegen-ia32.cc index ee1b92d09a..5e5d7c17a0 100644 --- a/src/ia32/fast-codegen-ia32.cc +++ b/src/ia32/fast-codegen-ia32.cc @@ -104,8 +104,19 @@ void FastCodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) { void FastCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) { Comment cmnt(masm_, "[ ReturnStatement"); SetStatementPosition(stmt); - Visit(stmt->expression()); - __ pop(eax); + Expression* expr = stmt->expression(); + Visit(expr); + + // Complete the statement based on the location of the subexpression. + Location source = expr->location(); + ASSERT(!source.is_nowhere()); + if (source.is_temporary()) { + __ pop(eax); + } else { + ASSERT(source.is_constant()); + ASSERT(expr->AsLiteral() != NULL); + __ mov(eax, expr->AsLiteral()->handle()); + } __ RecordJSReturn(); // Do not use the leave instruction here because it is too short to // patch with the code required by the debugger. @@ -132,30 +143,45 @@ void FastCodeGenerator::VisitVariableProxy(VariableProxy* expr) { } -void FastCodeGenerator::VisitLiteral(Literal* expr) { - Comment cmnt(masm_, "[ Literal"); - if (expr->location().is_temporary()) { - __ push(Immediate(expr->handle())); - } else { - ASSERT(expr->location().is_nowhere()); - } -} - - void FastCodeGenerator::VisitAssignment(Assignment* expr) { Comment cmnt(masm_, "[ Assignment"); ASSERT(expr->op() == Token::ASSIGN || expr->op() == Token::INIT_VAR); - Visit(expr->value()); + Expression* rhs = expr->value(); + Visit(rhs); + // Left-hand side is always a (parameter or local) slot. Variable* var = expr->target()->AsVariableProxy()->AsVariable(); ASSERT(var != NULL && var->slot() != NULL); - if (expr->location().is_temporary()) { - __ mov(eax, Operand(esp, 0)); - __ mov(Operand(ebp, SlotOffset(var->slot())), eax); + // Complete the assignment based on the location of the right-hand-side + // value and the desired location of the assignment value. + Location destination = expr->location(); + Location source = rhs->location(); + ASSERT(!destination.is_constant()); + ASSERT(!source.is_nowhere()); + + if (source.is_temporary()) { + if (destination.is_temporary()) { + // Case 'temp1 <- (var = temp0)'. Preserve right-hand-side temporary + // on the stack. + __ mov(eax, Operand(esp, 0)); + __ mov(Operand(ebp, SlotOffset(var->slot())), eax); + } else { + ASSERT(destination.is_nowhere()); + // Case 'var = temp'. Discard right-hand-side temporary. + __ pop(Operand(ebp, SlotOffset(var->slot()))); + } } else { - ASSERT(expr->location().is_nowhere()); - __ pop(Operand(ebp, SlotOffset(var->slot()))); + ASSERT(source.is_constant()); + ASSERT(rhs->AsLiteral() != NULL); + // Two cases: 'temp <- (var = constant)', or 'var = constant' with a + // discarded result. Always perform the assignment. + __ mov(eax, rhs->AsLiteral()->handle()); + __ mov(Operand(ebp, SlotOffset(var->slot())), eax); + if (destination.is_temporary()) { + // Case 'temp <- (var = constant)'. Save result. + __ push(eax); + } } } diff --git a/src/location.h b/src/location.h index 59cd88a070..9702ce4e3f 100644 --- a/src/location.h +++ b/src/location.h @@ -41,6 +41,7 @@ class Location BASE_EMBEDDED { bool is_temporary() { return type_ == TEMP; } bool is_nowhere() { return type_ == NOWHERE; } + bool is_constant() { return type_ == CONSTANT; } private: enum Type { TEMP, NOWHERE, CONSTANT }; diff --git a/src/x64/fast-codegen-x64.cc b/src/x64/fast-codegen-x64.cc index c43383631b..9dcdf8e8b8 100644 --- a/src/x64/fast-codegen-x64.cc +++ b/src/x64/fast-codegen-x64.cc @@ -112,8 +112,19 @@ void FastCodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) { void FastCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) { Comment cmnt(masm_, "[ ReturnStatement"); SetStatementPosition(stmt); - Visit(stmt->expression()); - __ pop(rax); + Expression* expr = stmt->expression(); + Visit(expr); + + // Complete the statement based on the location of the subexpression. + Location source = expr->location(); + ASSERT(!source.is_nowhere()); + if (source.is_temporary()) { + __ pop(rax); + } else { + ASSERT(source.is_constant()); + ASSERT(expr->AsLiteral() != NULL); + __ Move(rax, expr->AsLiteral()->handle()); + } __ RecordJSReturn(); // Do not use the leave instruction here because it is too short to // patch with the code required by the debugger. @@ -149,31 +160,45 @@ void FastCodeGenerator::VisitVariableProxy(VariableProxy* expr) { } -void FastCodeGenerator::VisitLiteral(Literal* expr) { - Comment cmnt(masm_, "[ Literal"); - if (expr->location().is_temporary()) { - __ Push(expr->handle()); - } else { - ASSERT(expr->location().is_nowhere()); - } -} - - void FastCodeGenerator::VisitAssignment(Assignment* expr) { Comment cmnt(masm_, "[ Assignment"); ASSERT(expr->op() == Token::ASSIGN || expr->op() == Token::INIT_VAR); + Expression* rhs = expr->value(); + Visit(rhs); - Visit(expr->value()); - + // Left-hand side is always a (parameter or local) slot. Variable* var = expr->target()->AsVariableProxy()->AsVariable(); ASSERT(var != NULL && var->slot() != NULL); - if (expr->location().is_temporary()) { - __ movq(rax, Operand(rsp, 0)); - __ movq(Operand(rbp, SlotOffset(var->slot())), rax); + // Complete the assignment based on the location of the right-hand-side + // value and the desired location of the assignment value. + Location destination = expr->location(); + Location source = rhs->location(); + ASSERT(!destination.is_constant()); + ASSERT(!source.is_nowhere()); + + if (source.is_temporary()) { + if (destination.is_temporary()) { + // Case 'temp1 <- (var = temp0)'. Preserve right-hand-side temporary + // on the stack. + __ movq(kScratchRegister, Operand(rsp, 0)); + __ movq(Operand(rbp, SlotOffset(var->slot())), kScratchRegister); + } else { + ASSERT(destination.is_nowhere()); + // Case 'var = temp'. Discard right-hand-side temporary. + __ pop(Operand(rbp, SlotOffset(var->slot()))); + } } else { - ASSERT(expr->location().is_nowhere()); - __ pop(Operand(rbp, SlotOffset(var->slot()))); + ASSERT(source.is_constant()); + ASSERT(rhs->AsLiteral() != NULL); + // Two cases: 'temp <- (var = constant)', or 'var = constant' with a + // discarded result. Always perform the assignment. + __ Move(kScratchRegister, rhs->AsLiteral()->handle()); + __ movq(Operand(rbp, SlotOffset(var->slot())), kScratchRegister); + if (destination.is_temporary()) { + // Case 'temp <- (var = constant)'. Save result. + __ push(kScratchRegister); + } } }