1141104 - Support INIT_THIN_INLINE in LSubstr

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P5)

Description

[Hannes Verschore [:h4writer]]

bug 1119081 comment 5

Note: I tried to use CreateDependentString in LSubstr when implementing. The only issue is that LSubstr is very constrained with it registers making it possible to squeeze some more perf. out of it. Using CreateDependentString would mean introducing an push/pop to create an extra register (in x86, since we are out of registers with sps enabled).

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: Patch

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 8574692 [details] [diff] [review]
Patch

Review of attachment 8574692 [details] [diff] [review]:
-----------------------------------------------------------------

I think I made a minor mistake. So clearing review for now.

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: Patch

Test for INIT_FAT_INLINE_FLAGS instead of INIT_THIN_INLINE_FLAGS, since INIT_FAT_INLINE_FLAGS = INIT_THIN_INLINE_FLAGS | JS_BIT(4)

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8574724 [details] [diff] [review]
Patch

Review of attachment 8574724 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/CodeGenerator.cpp
@@ +5884,5 @@
>      masm.branchTest32(Assembler::Zero, stringFlags, Imm32(JSString::INLINE_CHARS_BIT), &notInline);
> +
> +    Label createThinInline, next;
> +    masm.branchTest32(Assembler::Zero, stringFlags, Imm32(JSString::INIT_FAT_INLINE_FLAGS),
> +                      &createThinInline);

If we have a thin inline string, this will be NonZero as well because some of the flags in INIT_FAT_INLINE_FLAGS will be set. I think it'd be nice to use the length-register, because a substring of a non-inline string will often fit in an inline string and a FatInlineString will often have a ThinInlineString substring. I didn't check but I expect that's what the C++ code does too.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1141104 - Use inline strings more in LSubstr codegen to match SubstringKernel better . r?anba!

We were allocating fat inline strings iff the base string is an inline string.

The patch changes this to be based on the length instead, so that we use inline
strings in a lot more cases. We now also allocate thin inline strings when possible.
This matches the C++ version of this code better.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dddf6ad9ce42
Use inline strings more in LSubstr codegen to match SubstringKernel better . r=anba

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/dddf6ad9ce42