From 882055ff6a58f6b585575229f40f364e5f2a3ad0 Mon Sep 17 00:00:00 2001 From: jkummerow Date: Wed, 17 Jun 2015 04:58:17 -0700 Subject: [PATCH] Clean up JSConstructStub - fix truthfulness of comments - use InitializeFieldsWithFiller more consistently - use unsigned comparisons for pointers No change in functionality intended. Bonus: improve JavaScriptFrame::Print() for an enhanced debugging experience: - print PC of each frame - print the function's source also for optimized frames Review URL: https://codereview.chromium.org/1186823003 Cr-Commit-Position: refs/heads/master@{#29082} --- src/arm/builtins-arm.cc | 15 ++++-------- src/arm/macro-assembler-arm.cc | 4 ++-- src/arm64/builtins-arm64.cc | 2 +- src/frames.cc | 34 +++++++++++++++++++--------- src/ia32/builtins-ia32.cc | 14 +++--------- src/ia32/macro-assembler-ia32.cc | 2 +- src/mips/builtins-mips.cc | 25 ++++++++------------ src/mips/macro-assembler-mips.cc | 4 ++-- src/mips64/builtins-mips64.cc | 25 ++++++++------------ src/mips64/macro-assembler-mips64.cc | 2 +- src/x64/builtins-x64.cc | 14 +++--------- src/x64/macro-assembler-x64.cc | 2 +- src/x87/macro-assembler-x87.cc | 2 +- 13 files changed, 60 insertions(+), 85 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index 2638033cde..faf27861f8 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -446,7 +446,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // initial map and properties and elements are set to empty fixed array. // r1: constructor function // r2: initial map - // r3: object size (not including memento if create_memento) + // r3: object size (including memento if create_memento) // r4: JSObject (not tagged) __ LoadRoot(r6, Heap::kEmptyFixedArrayRootIndex); __ mov(r5, r4); @@ -520,7 +520,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ add(r4, r4, Operand(kHeapObjectTag)); // Check if a non-empty properties array is needed. Continue with - // allocated object if not fall through to runtime call if it is. + // allocated object if not; allocate and initialize a FixedArray if yes. // r1: constructor function // r4: JSObject // r5: start of next object (not tagged) @@ -575,15 +575,8 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // r5: FixedArray (not tagged) __ add(r6, r2, Operand(r3, LSL, kPointerSizeLog2)); // End of object. DCHECK_EQ(2 * kPointerSize, FixedArray::kHeaderSize); - { Label loop, entry; - __ LoadRoot(r0, Heap::kUndefinedValueRootIndex); - __ b(&entry); - __ bind(&loop); - __ str(r0, MemOperand(r2, kPointerSize, PostIndex)); - __ bind(&entry); - __ cmp(r2, r6); - __ b(lt, &loop); - } + __ LoadRoot(r0, Heap::kUndefinedValueRootIndex); + __ InitializeFieldsWithFiller(r2, r6, r0); // Store the initialized FixedArray into the properties field of // the JSObject diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index c7452cf3b1..61e484bd85 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -3174,7 +3174,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, str(filler, MemOperand(start_offset, kPointerSize, PostIndex)); bind(&entry); cmp(start_offset, end_offset); - b(lt, &loop); + b(lo, &loop); } @@ -3402,7 +3402,7 @@ void MacroAssembler::CallCFunctionHelper(Register function, if (ActivationFrameAlignment() > kPointerSize) { ldr(sp, MemOperand(sp, stack_passed_arguments * kPointerSize)); } else { - add(sp, sp, Operand(stack_passed_arguments * sizeof(kPointerSize))); + add(sp, sp, Operand(stack_passed_arguments * kPointerSize)); } } diff --git a/src/arm64/builtins-arm64.cc b/src/arm64/builtins-arm64.cc index b70500a41e..4a679789a5 100644 --- a/src/arm64/builtins-arm64.cc +++ b/src/arm64/builtins-arm64.cc @@ -522,7 +522,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ Add(new_obj, new_obj, kHeapObjectTag); // Check if a non-empty properties array is needed. Continue with - // allocated object if not, or fall through to runtime call if it is. + // allocated object if not; allocate and initialize a FixedArray if yes. Register element_count = x3; __ Ldrb(element_count, FieldMemOperand(init_map, Map::kUnusedPropertyFieldsOffset)); diff --git a/src/frames.cc b/src/frames.cc index c031971d5a..c6b04f7081 100644 --- a/src/frames.cc +++ b/src/frames.cc @@ -1120,6 +1120,24 @@ void StackFrame::PrintIndex(StringStream* accumulator, } +namespace { + + +void PrintFunctionSource(StringStream* accumulator, SharedFunctionInfo* shared, + Code* code) { + if (FLAG_max_stack_trace_source_length != 0 && code != NULL) { + std::ostringstream os; + os << "--------- s o u r c e c o d e ---------\n" + << SourceCodeOf(shared, FLAG_max_stack_trace_source_length) + << "\n-----------------------------------------\n"; + accumulator->Add(os.str().c_str()); + } +} + + +} // namespace + + void JavaScriptFrame::Print(StringStream* accumulator, PrintMode mode, int index) const { @@ -1157,7 +1175,7 @@ void JavaScriptFrame::Print(StringStream* accumulator, accumulator->Add(":~%d", line); } - accumulator->Add("] "); + accumulator->Add("] [pc=%p] ", pc); } accumulator->Add("(this=%o", receiver); @@ -1182,7 +1200,9 @@ void JavaScriptFrame::Print(StringStream* accumulator, return; } if (is_optimized()) { - accumulator->Add(" {\n// optimized frame\n}\n"); + accumulator->Add(" {\n// optimized frame\n"); + PrintFunctionSource(accumulator, shared, code); + accumulator->Add("}\n"); return; } accumulator->Add(" {\n"); @@ -1249,15 +1269,7 @@ void JavaScriptFrame::Print(StringStream* accumulator, accumulator->Add(" [%02d] : %o\n", i, GetExpression(i)); } - // Print details about the function. - if (FLAG_max_stack_trace_source_length != 0 && code != NULL) { - std::ostringstream os; - SharedFunctionInfo* shared = function->shared(); - os << "--------- s o u r c e c o d e ---------\n" - << SourceCodeOf(shared, FLAG_max_stack_trace_source_length) - << "\n-----------------------------------------\n"; - accumulator->Add(os.str().c_str()); - } + PrintFunctionSource(accumulator, shared, code); accumulator->Add("}\n\n"); } diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index 6cbac53003..8157b27666 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -358,17 +358,9 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // ebx: JSObject // edi: FixedArray // ecx: start of next object - { Label loop, entry; - __ mov(edx, factory->undefined_value()); - __ lea(eax, Operand(edi, FixedArray::kHeaderSize)); - __ jmp(&entry); - __ bind(&loop); - __ mov(Operand(eax, 0), edx); - __ add(eax, Immediate(kPointerSize)); - __ bind(&entry); - __ cmp(eax, ecx); - __ j(below, &loop); - } + __ mov(edx, factory->undefined_value()); + __ lea(eax, Operand(edi, FixedArray::kHeaderSize)); + __ InitializeFieldsWithFiller(eax, ecx, edx); // Store the initialized FixedArray into the properties field of // the JSObject diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index c445b5edf9..6e43c485fc 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -1751,7 +1751,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, add(start_offset, Immediate(kPointerSize)); bind(&entry); cmp(start_offset, end_offset); - j(less, &loop); + j(below, &loop); } diff --git a/src/mips/builtins-mips.cc b/src/mips/builtins-mips.cc index e20a408d7e..8381709f8d 100644 --- a/src/mips/builtins-mips.cc +++ b/src/mips/builtins-mips.cc @@ -452,7 +452,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // initial map and properties and elements are set to empty fixed array. // a1: constructor function // a2: initial map - // a3: object size (not including memento if create_memento) + // a3: object size (including memento if create_memento) // t4: JSObject (not tagged) __ LoadRoot(t6, Heap::kEmptyFixedArrayRootIndex); __ mov(t5, t4); @@ -532,7 +532,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ Addu(t4, t4, Operand(kHeapObjectTag)); // Check if a non-empty properties array is needed. Continue with - // allocated object if not fall through to runtime call if it is. + // allocated object if not; allocate and initialize a FixedArray if yes. // a1: constructor function // t4: JSObject // t5: start of next object (not tagged) @@ -568,7 +568,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // a1: constructor // a3: number of elements in properties array (untagged) // t4: JSObject - // t5: start of next object + // t5: start of FixedArray (untagged) __ LoadRoot(t6, Heap::kFixedArrayMapRootIndex); __ mov(a2, t5); __ sw(t6, MemOperand(a2, JSObject::kMapOffset)); @@ -588,20 +588,13 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ sll(t3, a3, kPointerSizeLog2); __ addu(t6, a2, t3); // End of object. DCHECK_EQ(2 * kPointerSize, FixedArray::kHeaderSize); - { Label loop, entry; - if (!is_api_function || create_memento) { - __ LoadRoot(t7, Heap::kUndefinedValueRootIndex); - } else if (FLAG_debug_code) { - __ LoadRoot(t2, Heap::kUndefinedValueRootIndex); - __ Assert(eq, kUndefinedValueNotLoaded, t7, Operand(t2)); - } - __ jmp(&entry); - __ bind(&loop); - __ sw(t7, MemOperand(a2)); - __ addiu(a2, a2, kPointerSize); - __ bind(&entry); - __ Branch(&loop, less, a2, Operand(t6)); + if (!is_api_function || create_memento) { + __ LoadRoot(t7, Heap::kUndefinedValueRootIndex); + } else if (FLAG_debug_code) { + __ LoadRoot(t2, Heap::kUndefinedValueRootIndex); + __ Assert(eq, kUndefinedValueNotLoaded, t7, Operand(t2)); } + __ InitializeFieldsWithFiller(a2, t6, t7); // Store the initialized FixedArray into the properties field of // the JSObject. diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc index 0b2bbf85c9..f554b0c1ef 100644 --- a/src/mips/macro-assembler-mips.cc +++ b/src/mips/macro-assembler-mips.cc @@ -3804,7 +3804,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, sw(filler, MemOperand(start_offset)); Addu(start_offset, start_offset, kPointerSize); bind(&entry); - Branch(&loop, lt, start_offset, Operand(end_offset)); + Branch(&loop, ult, start_offset, Operand(end_offset)); } @@ -5567,7 +5567,7 @@ void MacroAssembler::CallCFunctionHelper(Register function, if (base::OS::ActivationFrameAlignment() > kPointerSize) { lw(sp, MemOperand(sp, stack_passed_arguments * kPointerSize)); } else { - Addu(sp, sp, Operand(stack_passed_arguments * sizeof(kPointerSize))); + Addu(sp, sp, Operand(stack_passed_arguments * kPointerSize)); } } diff --git a/src/mips64/builtins-mips64.cc b/src/mips64/builtins-mips64.cc index 028d5651c6..ac13156865 100644 --- a/src/mips64/builtins-mips64.cc +++ b/src/mips64/builtins-mips64.cc @@ -453,7 +453,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // initial map and properties and elements are set to empty fixed array. // a1: constructor function // a2: initial map - // a3: object size (not including memento if create_memento) + // a3: object size (including memento if create_memento) // t0: JSObject (not tagged) __ LoadRoot(t2, Heap::kEmptyFixedArrayRootIndex); __ mov(t1, t0); @@ -535,7 +535,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ Daddu(t0, t0, Operand(kHeapObjectTag)); // Check if a non-empty properties array is needed. Continue with - // allocated object if not fall through to runtime call if it is. + // allocated object if not; allocate and initialize a FixedArray if yes. // a1: constructor function // t0: JSObject // t1: start of next object (not tagged) @@ -574,7 +574,7 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // a1: constructor // a3: number of elements in properties array (untagged) // t0: JSObject - // t1: start of next object + // t1: start of FixedArray (untagged) __ LoadRoot(t2, Heap::kFixedArrayMapRootIndex); __ mov(a2, t1); __ sd(t2, MemOperand(a2, JSObject::kMapOffset)); @@ -595,20 +595,13 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, __ dsll(a7, a3, kPointerSizeLog2); __ daddu(t2, a2, a7); // End of object. DCHECK_EQ(2 * kPointerSize, FixedArray::kHeaderSize); - { Label loop, entry; - if (!is_api_function || create_memento) { - __ LoadRoot(t3, Heap::kUndefinedValueRootIndex); - } else if (FLAG_debug_code) { - __ LoadRoot(a6, Heap::kUndefinedValueRootIndex); - __ Assert(eq, kUndefinedValueNotLoaded, t3, Operand(a6)); - } - __ jmp(&entry); - __ bind(&loop); - __ sd(t3, MemOperand(a2)); - __ daddiu(a2, a2, kPointerSize); - __ bind(&entry); - __ Branch(&loop, less, a2, Operand(t2)); + if (!is_api_function || create_memento) { + __ LoadRoot(t3, Heap::kUndefinedValueRootIndex); + } else if (FLAG_debug_code) { + __ LoadRoot(a6, Heap::kUndefinedValueRootIndex); + __ Assert(eq, kUndefinedValueNotLoaded, t3, Operand(a6)); } + __ InitializeFieldsWithFiller(a2, t2, t3); // Store the initialized FixedArray into the properties field of // the JSObject. diff --git a/src/mips64/macro-assembler-mips64.cc b/src/mips64/macro-assembler-mips64.cc index 6a947e1c1a..f7a77dd1b1 100644 --- a/src/mips64/macro-assembler-mips64.cc +++ b/src/mips64/macro-assembler-mips64.cc @@ -3857,7 +3857,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, sd(filler, MemOperand(start_offset)); Daddu(start_offset, start_offset, kPointerSize); bind(&entry); - Branch(&loop, lt, start_offset, Operand(end_offset)); + Branch(&loop, ult, start_offset, Operand(end_offset)); } diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index 7ec210ae8d..4fa405d55f 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -353,17 +353,9 @@ static void Generate_JSConstructStubHelper(MacroAssembler* masm, // rdi: FixedArray // rax: start of next object // rdx: number of elements - { Label loop, entry; - __ LoadRoot(rdx, Heap::kUndefinedValueRootIndex); - __ leap(rcx, Operand(rdi, FixedArray::kHeaderSize)); - __ jmp(&entry); - __ bind(&loop); - __ movp(Operand(rcx, 0), rdx); - __ addp(rcx, Immediate(kPointerSize)); - __ bind(&entry); - __ cmpp(rcx, rax); - __ j(below, &loop); - } + __ LoadRoot(rdx, Heap::kUndefinedValueRootIndex); + __ leap(rcx, Operand(rdi, FixedArray::kHeaderSize)); + __ InitializeFieldsWithFiller(rcx, rax, rdx); // Store the initialized FixedArray into the properties field of // the JSObject diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index 1485a24a2a..092b5bc83d 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -4570,7 +4570,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, addp(start_offset, Immediate(kPointerSize)); bind(&entry); cmpp(start_offset, end_offset); - j(less, &loop); + j(below, &loop); } diff --git a/src/x87/macro-assembler-x87.cc b/src/x87/macro-assembler-x87.cc index 369ee128ae..46c1830c05 100644 --- a/src/x87/macro-assembler-x87.cc +++ b/src/x87/macro-assembler-x87.cc @@ -1717,7 +1717,7 @@ void MacroAssembler::InitializeFieldsWithFiller(Register start_offset, add(start_offset, Immediate(kPointerSize)); bind(&entry); cmp(start_offset, end_offset); - j(less, &loop); + j(below, &loop); }