1041781 - IonMonkey: Optimize constant charCodeAt.

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Kannan Vijayan [:djvj]]

I was taking a stab at writing an efficient tokenizer in JS over the weekend, and I realized that there's no efficient way of matching input codepoints against pattern codepoints that are both constant and legible.

For example:

var c = this.nextChar();
if (c == 'x'.charCodeAt(0)) {
  ...
}

Is slow and forces a string lookup.  Putting the property on an object is better, but it forces a property load.  Neither case allows the placement for a constant number that is clearly derived from its character representation.

Ion can optimize these just fine.  MCall("charCodeAt", MConstantString, MConstantInt32) can reduce to a constant int32 result.  This lets us use

C.charCodeAt(0) as a shorthand for the integer code for a char C.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: optimize-constant-charcodeat.patch

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 8459844 [details] [diff] [review]
optimize-constant-charcodeat.patch

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

Oh that is sad we can't use GVN for this. Since we cannot access JSString from another thread.
Though I have some questions:

1) Can you move this to under the checks if inlining will work or not. So after callInfo.setImplicitlyUsedUnchecked();
2) Can you fallback to MCharCodeAt instead of returning InliningStatus_NotInlined;

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: inline-constant-charcodeat.patch

Comments addressed.  Running on try: https://tbpl.mozilla.org/?tree=Try&rev=81060beb8d90

Comment 4

[Hannes Verschore [:h4writer]]

Comment on attachment 8465516 [details] [diff] [review]
inline-constant-charcodeat.patch

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

Nice. Using a new function is even better.
Now I retract one of the comments I made. Please move the callInfo.setImplicitlyUsedUnchecked();.
Because all functions in there do:

function () {
   check_if_can_inline

   callInfo.setImplicitlyUsedUnchecked();

   inline code

}

::: js/src/jit/MCallOptimize.cpp
@@ +1178,5 @@
>      MIRType argType = callInfo.getArg(0)->type();
>      if (argType != MIRType_Int32 && argType != MIRType_Double)
>          return InliningStatus_NotInlined;
>  
>      callInfo.setImplicitlyUsedUnchecked();

Can you move this under inlineConstantCharCodeAt()

@@ +1208,5 @@
> +        return InliningStatus_NotInlined;
> +
> +    if (!callInfo.getArg(0)->isConstant())
> +        return InliningStatus_NotInlined;
> +

+ callInfo.setImplicitlyUsedUnchecked();

Comment 5

[Kannan Vijayan [:djvj]]

Comments addressed.  Pushed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8da59dd9fc7f

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8da59dd9fc7f

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8da59dd9fc7f