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}

diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc
index 2638033..faf2786 100644 (file)
--- 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 c7452cf..61e484b 100644 (file)
--- 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 b70500a..4a67978 100644 (file)
--- 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 c031971..c6b04f7 100644 (file)
--- 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 6cbac53..8157b27 100644 (file)
--- 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 c445b5e..6e43c48 100644 (file)
--- 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 e20a408..8381709 100644 (file)
--- 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 0b2bbf8..f554b0c 100644 (file)
--- 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 028d565..ac13156 100644 (file)
--- 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 6a947e1..f7a77dd 100644 (file)
--- 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 7ec210a..4fa405d 100644 (file)
--- 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 1485a24..092b5bc 100644 (file)
--- 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 369ee12..46c1830 100644 (file)
--- 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);
 }