Closed Bug 1265748 Opened 4 years ago Closed 4 years ago

non-Object path in IonBuilder::inlineIsCallable is not used.

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(1 file)

> IonBuilder::InliningStatus
> IonBuilder::inlineIsCallable(CallInfo& callInfo)
> {
> ...
>     if (callInfo.getArg(0)->type() != MIRType_Object)
>         return InliningStatus_NotInlined;
> 
>     // Try inlining with constant true/false: only objects may be callable at
>     // all, and if we know the class check if it is callable.
>     bool isCallableKnown = false;
>     bool isCallableConstant;
>     if (callInfo.getArg(0)->type() != MIRType_Object) {
>         isCallableKnown = true;
>         isCallableConstant = false;
>     } else {
>     ...
> }

2nd |callInfo.getArg(0)->type() != MIRType_Object| check won't become true.
IsCallable should be known to be false if the argument is primitive (including undefined and null).
It's not inlined if |type() > MIRType_Object|, like if MIRType_Value.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3115a732376f
Assignee: nobody → arai.unmht
Attachment #8743110 - Flags: review?(shu)
Comment on attachment 8743110 [details] [diff] [review]
Enable non-Object path in IonBuilder::inlineIsCallable.

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

Wow that logic was broken. Thanks for the patch, and nice test.

::: js/src/jit/MCallOptimize.cpp
@@ +2661,5 @@
>      if (getInlineReturnType() != MIRType_Boolean)
>          return InliningStatus_NotInlined;
> +
> +    MDefinition* arg = callInfo.getArg(0);
> +    if (arg->type() > MIRType_Object)

Could use a comment on what it means for the type to be > MIRType_Object.
Attachment #8743110 - Flags: review?(shu) → review+
Comment on attachment 8743110 [details] [diff] [review]
Enable non-Object path in IonBuilder::inlineIsCallable.

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

::: js/src/jit/MCallOptimize.cpp
@@ +2661,5 @@
>      if (getInlineReturnType() != MIRType_Boolean)
>          return InliningStatus_NotInlined;
> +
> +    MDefinition* arg = callInfo.getArg(0);
> +    if (arg->type() > MIRType_Object)

Also make a similar comment change in IonTypes.h, unless enum MIRType on trunk doesn't have such a comment.  (And I'd probably put the comment inline in the enum initializer list, rather than [potentially] out of context atop the enum.)
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b609cab107795bd79520e1674ddfdee80e46a8a
Bug 1265748 - Enable non-Object path in IonBuilder::inlineIsCallable. r=shu
https://hg.mozilla.org/mozilla-central/rev/0b609cab1077
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.