MIPS64: Improve long branches utilizing code range.

MIPS64: Improve long branches utilizing code range.

Improves code size of generated regexp in TestSizeOfRegExpCode test by 33%.

Execution time of the same test improved by ~10%.

Utilizing code range for mips64 enable us to use J/JAL
instructions for long branches.

TEST=cctest/test-heap/TestSizeOfRegExpCode
BUG=

Committed: https://crrev.com/962703cac042b5208e809a462b8bfa1722d38359
Cr-Commit-Position: refs/heads/master@{#28867}

[dusmil.imgtec, 2015-05-22 09:59:16]

dusan.milosavljevic@imgtec.com changed reviewers:
+ akos.palfi@imgtec.com, balazs.kilvady@imgtec.com, jkummerow@chromium.org,
jochen@chromium.org, paul.lind@imgtec.com, ulan@chromium.org

[dusmil.imgtec, 2015-05-25 09:08:58]

PTAL. This needs approval from v8 team.

[akos.palfi.imgtec, 2015-05-25 10:25:06]

MIPS part LGTM.

[jochen (gone - plz use gerrit), 2015-05-26 15:44:35]

https://codereview.chromium.org/1147503002/diff/60001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1147503002/diff/60001/src/globals.h#newcode145
src/globals.h:145: #if V8_TARGET_ARCH_MIPS64
can you explain why this is needed?

https://codereview.chromium.org/1147503002/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1147503002/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:117: #ifdef V8_TARGET_ARCH_MIPS64
same here?

[dusmil.imgtec, 2015-05-26 16:16:34]

https://codereview.chromium.org/1147503002/diff/60001/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/1147503002/diff/60001/src/globals.h#newcode145
src/globals.h:145: #if V8_TARGET_ARCH_MIPS64
On 2015/05/26 15:44:35, jochen wrote:
> can you explain why this is needed?

Done.

https://codereview.chromium.org/1147503002/diff/60001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1147503002/diff/60001/src/heap/spaces.cc#newc...
src/heap/spaces.cc:117: #ifdef V8_TARGET_ARCH_MIPS64
On 2015/05/26 15:44:35, jochen wrote:
> same here?

Done.

[jochen (gone - plz use gerrit), 2015-05-27 14:48:26]

hum, so V8 is not unlikely to run out of code space memory and crash. Is there
an indication that this optimization will save us enough space to offset the
higher risk of crashing?

From the CL description, it sounds like we'll save 33% on code generated for
regexps, but we have overall 50% less space for code?

[dusmil.imgtec, 2015-05-28 13:45:19]

Not quite, this is already checked on octane and asm.js tests,
and we have margins over 100MB free space for code space.

Yes, there is concern for extreme JS code size in future and we are aware of
that.
Also we have strategy for this problem. It is only matter of decision whether we
limit code size to 256mb and use J/JAL instructions for long branches which will
also give us opportunity to use them for calls, or use them for long branches
only within code objects which does not require code range limiting to 256MB.

In later above-mentioned approach I can tweak code range allocator to be
"unlimited" (limited to 512mb as for x64), but it has to take care to not
allocate code objects across
256-MB regions for mips64 only. I can add this in next patch set or next CL,
therefore please approve this CL if you think memory allocator tweaks is more
suitable for next CL, otherwise I will create next patch set.

[jochen (gone - plz use gerrit), 2015-05-28 14:55:47]

how is a 256mb region defined? would all code objects have to be in that 256mb
region?

as I mentioned, when running in a browser, we get quite close tot he 512mb limit
and often exceed it, so a solution that does not depend on restricting the code
space size is preferable imo

[dusmil.imgtec, 2015-05-29 12:06:12]

Right, thanks for info about memory consumption in browser.

No, all code objects do not have to be in 256MB memory regions, but one object
must not be allocated across two 256MB regions. In other words start of code
object and its end have to be in the same region witch size and alignment is set
to 256MB. This is required so the absolute addresses encoded in 28-bit immediate
for jumps in j/jal instructions are correct.

This implies maximum code object size is limited to 256MB, but size of code
space is unlimited.

[Erik Corry, 2015-05-29 13:51:09]

erik.corry@gmail.com changed reviewers:
+ erik.corry@gmail.com

[Erik Corry, 2015-05-29 13:51:10]

Only large objects can span a 256Mbyte boundary, so an alternative
implementation would fix large-object allocation to reject allocations that span
such a boundary. In this scenario you would not need to reserve code space up
front and there would be n 256Mbyte limit on code size?

[dusmil.imgtec, 2015-06-01 11:32:42]

Exactly, thank you for pointing this out.

Yes, we do not need to do anything for code objects in code space.
Right, the limit would be n x 256MB.
Since large pages are being added one by one in linked list for LO space they
are scattered over virtual address space,
the code region should be reserved up front to handle position of 256MB
boundary. Otherwise, every large page allocation has to be aligned to 256MB
which will
have huge pressure on OS memory system.

[Erik Corry, 2015-06-01 12:45:48]

One solution would be to get large objects from the reserved code area, but
continue to get normal code pages from unreserved memory.  Then you are not
imposing a 256Mbyte code limit on the platform, but only a 256Mbyte limit on
large code objects (> 1Mbyte).

[dusmil.imgtec, 2015-06-02 10:35:42]

Right. New patch set uploaded. PTAL.

[dusmil.imgtec, 2015-06-04 10:06:51]

PTAL.

[Erik Corry, 2015-06-04 10:28:15]

https://codereview.chromium.org/1147503002/diff/100001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1147503002/diff/100001/src/heap/spaces.cc#new...
src/heap/spaces.cc:659: commit_size > CodePageAreaSize()) {
The commit_size includes the CodePageGuardStartOffset(), but the
CodePageAreaSize does not include it. It looks like you are comparing two
incomparable things here and I suspect it always takes normal pages from the
reserved area instead of allocating them individually.

https://codereview.chromium.org/1147503002/diff/100001/src/mips64/assembler-m...
File src/mips64/assembler-mips64.h (right):

https://codereview.chromium.org/1147503002/diff/100001/src/mips64/assembler-m...
src/mips64/assembler-mips64.h:1443: // If trampoline is emitted, generated code
is becoming large. As this is
Is this comment still accurate?

[dusmil.imgtec, 2015-06-04 11:04:41]

https://codereview.chromium.org/1147503002/diff/100001/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/1147503002/diff/100001/src/heap/spaces.cc#new...
src/heap/spaces.cc:659: commit_size > CodePageAreaSize()) {
Thank you for spotting this, typo mistake sorry. Too many variables with the
similar names.

https://codereview.chromium.org/1147503002/diff/100001/src/heap/spaces.cc#new...
src/heap/spaces.cc:659: commit_size > CodePageAreaSize()) {
On 2015/06/04 10:28:15, Erik Corry wrote:
> The commit_size includes the CodePageGuardStartOffset(), but the
> CodePageAreaSize does not include it. It looks like you are comparing two
> incomparable things here and I suspect it always takes normal pages from the
> reserved area instead of allocating them individually.

Done.

https://codereview.chromium.org/1147503002/diff/100001/src/mips64/assembler-m...
File src/mips64/assembler-mips64.h (right):

https://codereview.chromium.org/1147503002/diff/100001/src/mips64/assembler-m...
src/mips64/assembler-mips64.h:1443: // If trampoline is emitted, generated code
is becoming large. As this is
On 2015/06/04 10:28:15, Erik Corry wrote:
> Is this comment still accurate?

Done.

https://codereview.chromium.org/1147503002/diff/100001/src/mips64/assembler-m...
src/mips64/assembler-mips64.h:1443: // If trampoline is emitted, generated code
is becoming large. As this is
Yes, stale comment from old age.

[dusmil.imgtec, 2015-06-05 11:33:11]

PTAL.

[Erik Corry, 2015-06-05 11:35:47]

LGTM

[jochen (gone - plz use gerrit), 2015-06-05 12:20:55]

jochen@chromium.org changed reviewers:
+ hpayer@chromium.org

[jochen (gone - plz use gerrit), 2015-06-05 12:20:56]

Ulan or Hannes should approve this

[dusmil.imgtec, 2015-06-08 13:15:50]

Hannes, PTAL.

[Hannes Payer (out of office), 2015-06-08 21:22:58]

LGTM

[dusmil.imgtec, 2015-06-09 13:58:30]

The CQ bit was checked by dusan.milosavljevic@imgtec.com

[dusmil.imgtec, 2015-06-09 13:58:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
akos.palfi@imgtec.com
Link to the patchset: https://codereview.chromium.org/1147503002/#ps120001
(title: "Fixed typo, addressed stale comment.")

[commit-bot: I haz the power, 2015-06-09 13:58:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1147503002/120001

[commit-bot: I haz the power, 2015-06-09 14:50:33]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-06-09 14:50:46]

Patchset 7 (id:??) landed as
https://crrev.com/962703cac042b5208e809a462b8bfa1722d38359
Cr-Commit-Position: refs/heads/master@{#28867}