Bug 208207 - [JSC] Support delete by val/id IC on 32-bits
Summary: [JSC] Support delete by val/id IC on 32-bits
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Lima
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-25 10:04 PST by Caio Lima
Modified: 2020-05-13 05:17 PDT (History)
8 users (show)

See Also:


Attachments
WIP - Patch (28.49 KB, patch)
2020-05-06 15:12 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (28.51 KB, patch)
2020-05-11 11:45 PDT, Caio Lima
no flags Details | Formatted Diff | Diff
Patch (28.51 KB, patch)
2020-05-12 12:26 PDT, Caio Lima
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Lima 2020-02-25 10:04:38 PST
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.
Comment 1 Caio Lima 2020-05-06 15:12:55 PDT
Created attachment 398672 [details]
WIP - Patch
Comment 2 Caio Lima 2020-05-06 15:13:23 PDT
Comment on attachment 398672 [details]
WIP - Patch

Let's check EWS.
Comment 3 Caio Lima 2020-05-11 11:45:57 PDT
Created attachment 399036 [details]
Patch
Comment 4 Saam Barati 2020-05-11 18:34:40 PDT
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?
Comment 5 Caio Lima 2020-05-12 04:16:11 PDT
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.
Comment 6 Saam Barati 2020-05-12 11:29:21 PDT
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)
Comment 7 Caio Lima 2020-05-12 12:26:54 PDT
Created attachment 399153 [details]
Patch
Comment 8 Caio Lima 2020-05-12 12:34:43 PDT
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.
Comment 9 EWS 2020-05-13 05:16:40 PDT
Committed r261610: <https://trac.webkit.org/changeset/261610>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399153 [details].
Comment 10 Radar WebKit Bug Importer 2020-05-13 05:17:13 PDT
<rdar://problem/63180603>