Bug 208207 – [JSC] Support delete by val/id IC on 32-bits

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Caio Lima]

We are adding IC to delete_by_val and delete_by_id on https://bugs.webkit.org/show_bug.cgi?id=207522, but it doesn't include support for 32-bits architectures.

[Caio Lima]

Created attachment 398672 [details]
WIP - Patch

[Caio Lima]

Comment on attachment 398672 [details]
WIP - Patch

Let's check EWS.

[Caio Lima]

Created attachment 399036 [details]
Patch

[Saam Barati]

Comment on attachment 399036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399036&action=review

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1175
> +        JSValueOperand key(this, node->child2(), ManualOperandSpeculation);

do we ever pick anything besides UntypedUse? Where is our speculation?

[Caio Lima]

Comment on attachment 399036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399036&action=review

Thank you very much for the comments! I'm answering questions below.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1175
>> +        JSValueOperand key(this, node->child2(), ManualOperandSpeculation);
> 
> do we ever pick anything besides UntypedUse? Where is our speculation?

IIUC, we have rules to fix up to CellUse from fix up phase. The speculation is just above at line 1172.

[Saam Barati]

Comment on attachment 399036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399036&action=review

r=me

>>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1175
>>> +        JSValueOperand key(this, node->child2(), ManualOperandSpeculation);
>> 
>> do we ever pick anything besides UntypedUse? Where is our speculation?
> 
> IIUC, we have rules to fix up to CellUse from fix up phase. The speculation is just above at line 1172.

I see. This style is weird, we typically speculate after such a LOC. (I understand you're just refactoring the code here)

[Caio Lima]

Created attachment 399153 [details]
Patch

[Caio Lima]

Comment on attachment 399036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399036&action=review

Thank you very much for the review

>>>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:1175
>>>> +        JSValueOperand key(this, node->child2(), ManualOperandSpeculation);
>>> 
>>> do we ever pick anything besides UntypedUse? Where is our speculation?
>> 
>> IIUC, we have rules to fix up to CellUse from fix up phase. The speculation is just above at line 1172.
> 
> I see. This style is weird, we typically speculate after such a LOC. (I understand you're just refactoring the code here)

I agree. I fixed it.

[EWS]

Committed r261610: <https://trac.webkit.org/changeset/261610>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399153 [details].

[Radar WebKit Bug Importer]

<rdar://problem/63180603>