Bug 238349 - AI should not set the structure for ObjectCreate
Summary: AI should not set the structure for ObjectCreate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Justin Michaud
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-24 14:51 PDT by Justin Michaud
Modified: 2022-03-25 14:52 PDT (History)
8 users (show)

See Also:

Attachments
Patch (2.58 KB, patch)
2022-03-24 14:53 PDT, Justin Michaud 		saam: review-
Details | Formatted Diff | Diff
Patch (3.12 KB, patch)
2022-03-25 11:49 PDT, Justin Michaud 		saam: review-
Details | Formatted Diff | Diff
Patch (3.03 KB, patch)
2022-03-25 13:09 PDT, Justin Michaud 		no flags Details | Formatted Diff | Diff
[fast-cq] Patch (2.95 KB, patch)
2022-03-25 13:10 PDT, Justin Michaud 		saam: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Michaud 2022-03-24 14:51:36 PDT 
The AbstractInterpreter should not set the structure for ObjectCreate because it might change by the time the constant folding phase runs if the structure cache is cleared.
Comment 1 Justin Michaud 2022-03-24 14:53:50 PDT 
Created attachment 455689 [details]
Patch
Comment 2 Yusuke Suzuki 2022-03-24 14:59:41 PDT 
Comment on attachment 455689 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:-3147
> -                setForNode(node, structure);

We should continue setting a structure for nullPrototypeObjectStructure.
Comment 3 Saam Barati 2022-03-24 15:00:59 PDT 
Comment on attachment 455689 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3138
>                  break;

You can’t break here. You need the below code to run and set our type.

I also suggest reworking this patch to never even bother looking up a structure and just always mark things as shoyldTryCobstantFolding or instead to keep the code as it used to be
Comment 4 Radar WebKit Bug Importer 2022-03-25 09:53:04 PDT 
<rdar://problem/90842529>
Comment 5 Mark Lam 2022-03-25 11:35:06 PDT 
<rdar://problem/90838071>
Comment 6 Justin Michaud 2022-03-25 11:49:56 PDT 
Created attachment 455790 [details]
Patch
Comment 7 Yusuke Suzuki 2022-03-25 12:05:08 PDT 
Why not moving StructureCache from VM to JSGlobalObject?
This cache is used for objects' structures. Thus each structure has its tied JSGlobalObject.
Comment 8 Saam Barati 2022-03-25 12:43:33 PDT 
Comment on attachment 455790 [details]
Patch

We're discussing on slack a better approach
Comment 9 Justin Michaud 2022-03-25 13:09:08 PDT 
Created attachment 455795 [details]
Patch
Comment 10 Saam Barati 2022-03-25 13:10:06 PDT 
Comment on attachment 455795 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3131
> -                    didFoldClobberWorld();
> +                    clobberWorld();

this shouldn't change.
Comment 11 Justin Michaud 2022-03-25 13:10:28 PDT 
Created attachment 455796 [details]
[fast-cq] Patch
Comment 12 Saam Barati 2022-03-25 13:13:15 PDT 
Comment on attachment 455796 [details]
[fast-cq] Patch

r=me
Comment 13 Yusuke Suzuki 2022-03-25 13:15:04 PDT 
r=me too.
Comment 14 Mark Lam 2022-03-25 14:52:34 PDT 
Patch landed in r291891: <http://trac.webkit.org/r291891>.