Account Options

  1. Sign in
The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Google Groups Home
« Groups Home
DoNumberTagD performance improvement (issue 11028115)
There are currently too many topics in this group that display first. To make this topic appear first, remove this option from another topic.
There was an error processing your request. Please try again.
flag
  5 messages - Collapse all  -  Translate all to Translated (View all originals)
The group you are posting to is a Usenet group. Messages posted to this group will make your email address visible to anyone on the Internet.
Your reply message has not been sent.
Your post was successful
 
From:
To:
Cc:
Followup To:
Add Cc | Add Followup-to | Edit Subject
Subject:
Validation:
For verification purposes please type the characters you see in the picture below or the numbers you hear by clicking the accessibility icon. Listen and type the numbers you hear
 
aber...@chromium.org  
View profile  
 More options Oct 10 2012, 7:28 am
From: aber...@chromium.org
Date: Wed, 10 Oct 2012 11:28:24 +0000
Local: Wed, Oct 10 2012 7:28 am
Subject: DoNumberTagD performance improvement (issue 11028115)
Reviewers: danno,

Message:
PTAL

Description:
DoNumberTagD performance improvement

Allocate heap entry untagged and tag at end to avoid having to subtract off
the tag offset before storing the value.

BUG=

Please review this at https://codereview.chromium.org/11028115/

SVN Base: git://github.com/v8/v8.git@master

Affected files:
   M src/arm/lithium-codegen-arm.cc
   M src/arm/macro-assembler-arm.h
   M src/arm/macro-assembler-arm.cc

Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index  
b5a0f9af31919d948e1ec2fa0fc6f77e47c5013f..52864d3c2b58625ff89bfe3ccb0fbefe9 2b6f831  
100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -4491,13 +4491,15 @@ void LCodeGen::DoNumberTagD(LNumberTagD* instr) {
    DeferredNumberTagD* deferred = new(zone()) DeferredNumberTagD(this,  
instr);
    if (FLAG_inline_new) {
      __ LoadRoot(scratch, Heap::kHeapNumberMapRootIndex);
-    __ AllocateHeapNumber(reg, temp1, temp2, scratch, deferred->entry());
+    // We want the untagged address first for performance
+    __ AllocateHeapNumber(reg, temp1, temp2, scratch, deferred->entry(),  
false);
    } else {
      __ jmp(deferred->entry());
    }
    __ bind(deferred->exit());
-  __ sub(ip, reg, Operand(kHeapObjectTag));
-  __ vstr(input_reg, ip, HeapNumber::kValueOffset);
+  __ vstr(input_reg, reg, HeapNumber::kValueOffset);
+  // Now that we have finished with the object's real address tag it
+  __ add(reg, reg, Operand(kHeapObjectTag));
  }

@@ -4510,6 +4512,7 @@ void LCodeGen::DoDeferredNumberTagD(LNumberTagD*  
instr) {

    PushSafepointRegistersScope scope(this, Safepoint::kWithRegisters);
    CallRuntimeFromDeferred(Runtime::kAllocateHeapNumber, 0, instr);
+  __ sub(r0, r0, Operand(kHeapObjectTag));
    __ StoreToSafepointRegisterSlot(r0, reg);
  }

Index: src/arm/macro-assembler-arm.cc
diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index  
cc7fc4d3a4203706ce0a9cccee63e43d2adbdbb2..a3cb02b4f38338c4bd9de6e642bc4321a 7ea3436  
100644
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -3122,7 +3122,8 @@ void MacroAssembler::AllocateHeapNumber(Register  
result,
                                          Register scratch1,
                                          Register scratch2,
                                          Register heap_number_map,
-                                        Label* gc_required) {
+                                        Label* gc_required,
+                                        bool tagged) {
    // Allocate an object in the heap for the heap number and tag it as a  
heap
    // object.
    AllocateInNewSpace(HeapNumber::kSize,
@@ -3130,11 +3131,15 @@ void MacroAssembler::AllocateHeapNumber(Register  
result,
                       scratch1,
                       scratch2,
                       gc_required,
-                     TAG_OBJECT);
+                     tagged?TAG_OBJECT:NO_ALLOCATION_FLAGS);

    // Store heap number map in the allocated object.
    AssertRegisterIsRoot(heap_number_map, Heap::kHeapNumberMapRootIndex);
-  str(heap_number_map, FieldMemOperand(result, HeapObject::kMapOffset));
+  if (tagged) {
+    str(heap_number_map, FieldMemOperand(result, HeapObject::kMapOffset));
+  } else {
+    str(heap_number_map, MemOperand(result, HeapObject::kMapOffset));
+  }
  }

Index: src/arm/macro-assembler-arm.h
diff --git a/src/arm/macro-assembler-arm.h b/src/arm/macro-assembler-arm.h
index  
ff0deef0f46dff25a17eb5cbae7b1ae5c3b0f47b..448259f735322f2f0d14d15b0b84e9fe2 b427eec  
100644
--- a/src/arm/macro-assembler-arm.h
+++ b/src/arm/macro-assembler-arm.h
@@ -731,7 +731,8 @@ class MacroAssembler: public Assembler {
                            Register scratch1,
                            Register scratch2,
                            Register heap_number_map,
-                          Label* gc_required);
+                          Label* gc_required,
+                          bool tagged = true);
    void AllocateHeapNumberWithValue(Register result,
                                     DwVfpRegister value,
                                     Register scratch1,


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
da...@chromium.org  
View profile  
 More options Oct 10 2012, 8:27 am
From: da...@chromium.org
Date: Wed, 10 Oct 2012 12:27:53 +0000
Local: Wed, Oct 10 2012 8:27 am
Subject: Re: DoNumberTagD performance improvement (issue 11028115)
LGTM with nits pending performance numbers

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
src/arm/macro-assembler-arm.cc:3134:
tagged?TAG_OBJECT:NO_ALLOCATION_FLAGS);
Spaces between ? and :

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
src/arm/macro-assembler-arm.h:735: bool tagged = true);
Use an enum here, like "enum TaggingMode { TAG_RESULT, DONT_TAG_RESULT

}" , otherwise at the call site it's difficult to tell what the

true/false actually means

http://codereview.chromium.org/11028115/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
aber...@chromium.org  
View profile  
 More options Oct 11 2012, 7:25 am
From: aber...@chromium.org
Date: Thu, 11 Oct 2012 11:25:39 +0000
Local: Thurs, Oct 11 2012 7:25 am
Subject: Re: DoNumberTagD performance improvement (issue 11028115)
 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
da...@chromium.org  
View profile  
 More options Oct 11 2012, 9:04 am
From: da...@chromium.org
Date: Thu, 11 Oct 2012 13:03:57 +0000
Local: Thurs, Oct 11 2012 9:03 am
Subject: Re: DoNumberTagD performance improvement (issue 11028115)
Almost there...

http://codereview.chromium.org/11028115/diff/6001/src/arm/lithium-cod...
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/11028115/diff/6001/src/arm/lithium-cod...
src/arm/lithium-codegen-arm.cc:4692:
ASSERT(input->Equals(instrt->result()));
I think the above change is a typo.

http://codereview.chromium.org/11028115/diff/6001/src/arm/stub-cache-...
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/11028115/diff/6001/src/arm/stub-cache-...
src/arm/stub-cache-arm.cc:3809:
nit: remove introduced whitespace

http://codereview.chromium.org/11028115/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
aber...@chromium.org  
View profile  
 More options Oct 11 2012, 11:16 am
From: aber...@chromium.org
Date: Thu, 11 Oct 2012 15:16:53 +0000
Local: Thurs, Oct 11 2012 11:16 am
Subject: Re: DoNumberTagD performance improvement (issue 11028115)

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
src/arm/macro-assembler-arm.cc:3134:
tagged?TAG_OBJECT:NO_ALLOCATION_FLAGS);
On 2012/10/10 12:27:53, danno wrote:

> Spaces between ? and :

Done.

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.h (right):

http://codereview.chromium.org/11028115/diff/1/src/arm/macro-assemble...
src/arm/macro-assembler-arm.h:735: bool tagged = true);
On 2012/10/10 12:27:53, danno wrote:

> Use an enum here, like "enum TaggingMode { TAG_RESULT, DONT_TAG_RESULT
}" ,
> otherwise at the call site it's difficult to tell what the true/false
actually
> means

Done.

http://codereview.chromium.org/11028115/diff/6001/src/arm/lithium-cod...
File src/arm/lithium-codegen-arm.cc (right):

http://codereview.chromium.org/11028115/diff/6001/src/arm/lithium-cod...
src/arm/lithium-codegen-arm.cc:4692:
ASSERT(input->Equals(instrt->result()));
On 2012/10/11 13:03:57, danno wrote:

> I think the above change is a typo.

Done.

http://codereview.chromium.org/11028115/diff/6001/src/arm/stub-cache-...
File src/arm/stub-cache-arm.cc (right):

http://codereview.chromium.org/11028115/diff/6001/src/arm/stub-cache-...
src/arm/stub-cache-arm.cc:3809:
On 2012/10/11 13:03:57, danno wrote:

> nit: remove introduced whitespace

Done.

http://codereview.chromium.org/11028115/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
End of messages
« Back to Discussions « Newer topic     Older topic »