Bug 157215 – Unite *with_this opcodes with their non this version

• Component: JavaScriptCore
• Status: NEW

[Saam Barati]

This would probably require making *by_id and *by_val also take a this parameter.
It would also requiring changing ICs.

[Caio Lima]

Created attachment 287252 [details]
WIP get_by_id

This is just the first move on this Patch. I am planing separate into in 2 Patches one to merge get_by_id* and other to put_by_id*

In this version I am justing supporting the LLInt layer.

In LLint layer, I am considering that the property cache can happen as the "ident" being cached as prototype cache of thisValue. It is not enabled to Getters yet, but there is a Patch implementing this (https://bugs.webkit.org/show_bug.cgi?id=158083). Is this reasoning consistent?

My major doubt in Baseline is about PIC. Do we support it to Getter/Setters? If yes, how this mechanism work? If I am not wrong, usually the IC is a stub considering some observed types and loading the correspondent offset for each object, however getter e setter requires a function call, right?

[Caio Lima]

Created attachment 287617 [details]
WIP get_by_id PIC fixing

This version is just a prototype to enable the new version of get_by_id PIC support.

I am not going to keep the different methods like "operationGetByIdOptimizeWithThis" and the JITGetByIdGenerator new constructor in the final version, but I am doing this way because it is easier to debug without change all 4 layers at once.

I am justing enabling it to 64 bits version, but 32 bits support is coming soon.

As I mentioned before, LLInt change isn't adding much because we aren't inline caching accessors yet. However, I noticed that this update enable PIC in Baseline layer.

[Caio Lima]

Created attachment 287868 [details]
WIP get_by_id in DFG

This version is enabling the main cases of get_by_id in DFG layer. I added thisOperand as a child2() of GetById(Flush) nodes. When these nodes are compiled and the getter is inlined, we can set thisOperand as the this of inlined Frame.

What I am missing to finish get_by_id_merge is:

- Merge it in FTL layer;
- Check other cases with Polymorphic and Megamorphic calls;
- Merge get_by_id in x86_32; 

I am having some troubles to compile the following sample in FTL mode:

```
"use strict";
var Base = class Base {
    constructor() { this._name = "Name"; }
    get name() { return this._name; } // If this instead returns a static: return "Foo" things somewhat work.
    set name(x) { this._name = x; }
};

var Subclass = class Subclass extends Base {
    get name() { return super.name; }
};

function getterName(instance) {
    return instance.name;
}

var instance = new Subclass;
for (let i = 0; i < 1000000;i++)
    getterName(instance);
```
I have no idea yet, because I dumped the DFG and found CheckTierUpAtReturn(MustGen, W:SideState, Exits, bc#16). Going to investigate it more.

[Caio Lima]

Created attachment 288932 [details]
WIP 64bits support in all layers

This Patch is implementing the 64bit version of get_by_id merge. The idea is to put this as get_by_id operand and propagate it through Baseline, DFG and FTL. To avoid register usage when this == base, we check this case on  DFG and FTL.

The test is considering Polymorphic and Megamorphic IC. Also there are 2 conditions to be checked that is when --useAccessInline=(false|true). I would like to know if there is a way to set a JSC option from a test script, because this way I can improve the test to consider --useAccessInline=(false|true) directly from tests.

There are a lot of work to do yet and definitely I am going to split the *by_id in 4 patches.