1326437 - Use CacheIR GetName IC in Ion

Status: RESOLVED FIXED

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

Description

[Tom S [:evilpie]]

The GetName Ion ICs are quite different from the previous baseline ICs that I ported to CacheIR. We should check which of the conditions that Ion checks before attaching a stub are required. We should also port the TypeOfNoProperty stub to CacheIR.

Comment 1

[Jan de Mooij [:jandem]]

GetName and BindName are now the only IonCaches ICs left. GetName should be pretty easy to convert and should let us remove a lot of code. I'll probably work on this next week.

After that, BindName will be trivial.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Port TypeOfNoProperty to CacheIR

Quite straight-forward after factoring out LookupEnvironment.

I also noticed we don't have spew for GetName yet? I'll file a follow-up bug for that.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Don't shape guard non-extensible call objects

This ports an optimization from GenerateEnvironmentChainGuard in IonCaches.cpp: we don't need to shape guard call objects with a non-extensible scope. 

There's a similar optimization for non-configurable properties on the global, but I didn't port that because it means we have to fix up the BaselineInspector too. This code was added a long time ago and nowadays GetName isn't very hot in Ion so I don't think it matters much.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Port GetName IC to CacheIR

I also fixed some minor pre-existing issues.

 11 files changed, 162 insertions(+), 890 deletions(-)

Comment 5

[Tom S [:evilpie]]

Comment on attachment 8854393 [details] [diff] [review]
Part 1 - Port TypeOfNoProperty to CacheIR

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

As mentioned on IRC this is probably wrong in Ion too. GlobalObjects have a proto-chain that could contains this property, as well as a missing shape guard for it.

Comment 6

[Tom S [:evilpie]]

Comment on attachment 8854394 [details] [diff] [review]
Part 2 - Don't shape guard non-extensible call objects

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

Nice, this seems like the most salvageable part of ion to me.

::: js/src/jit/CacheIR.cpp
@@ +1824,5 @@
> +    // variables). The function might have been relazified under rare
> +    // conditions. In that case, we pessimistically create the guard.
> +    CallObject* callObj = &envObj->as<CallObject>();
> +    JSFunction* fun = &callObj->callee();
> +    if (fun->hasScript() && !fun->nonLazyScript()->funHasExtensibleScope())

Nit: Could be written as !fun->hasScript() || ...HasExtensibleScope()

Comment 7

[Tom S [:evilpie]]

Comment on attachment 8854395 [details] [diff] [review]
Part 3 - Port GetName IC to CacheIR

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

Looks great!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/406065489474
part 2 - Don't shape guard non-extensible call objects. r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/9927b6f58b07
part 3 - Port Ion GetName IC to CacheIR. r=evilpie

Comment 9

[Jan de Mooij [:jandem]]

I dropped part 1, see bug 1353384 comment 4.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/406065489474
https://hg.mozilla.org/mozilla-central/rev/9927b6f58b07