Summary: | Unite *with_this opcodes with their non this version | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||||
Status: | NEW --- | ||||||||||||
Severity: | Normal | CC: | benjamin, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, sukolsak, ysuzuki | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 162125, 162126, 162127, 162124 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Saam Barati
2016-04-29 16:49:54 PDT
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? 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.
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.
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.
|