Repository navigation
[JSC] AOT: iterators of String objects, long literals, loaders of built-in modules #903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 250 commits
0c6bb6a
cad6541
85996cb
8226361
eaeb582
ce441cf
4bf4fd1
413c7cc
d5e7760
f9dc315
fc299f0
7e4f367
138f015
b0da97c
53351da
c4b17ea
5d23bb9
8527409
4ec4351
ea2b24a
eaa6ea7
560d360
6a7b0e0
1cedad7
e674b78
6bcf989
f3478e7
2999517
b626bae
6d2ece5
3624f8c
c9a07ca
1e83a17
a8947ad
ce07c57
bb9f1e6
d5f69d0
ca74836
b13dad7
0ade91d
1551491
b556a1c
0a26932
2b899c1
5c200dd
2a82a17
2f8bb4d
18d77da
204f9ea
951d841
17008ff
af4d070
59ba842
2cd1e7f
4ac91d4
883bb54
6acbaec
a3e13d5
3276897
fb2d765
967800a
053953f
d8cf5c1
2eaa55d
33cdb5c
23d5def
b6ae515
84fd5d5
dc2a54d
c08baff
f3757ae
0e3a9c4
9edecf0
abd868b
794fd45
d6e167e
33f6335
f6996af
4516d82
72dde0f
b5be4ae
1ce629a
b683570
ed1e2a8
c9f02f1
c2bd67e
0030946
e416019
0db3f5e
a537a43
0da0a4a
a4e9bfd
da657fe
6227115
afe5bf6
3cbd28f
5698252
44cb391
8805107
7488000
ed6cada
bc15181
0ed1d74
0ae359e
f3a215c
cb17232
4008c5f
a2ef284
d2df562
a4f1b23
bf1070a
2b9b22d
fb07502
42c3d09
d70450c
61ae3e8
30b7133
f2df6d5
84c71e9
88050aa
25baf35
845b646
85e843a
2a26607
8813190
14197ac
b973b21
5bb9e4b
e6c5d9a
49220e7
6c16057
50dd4bf
b4ecf32
3c35c2b
398e876
58f2ff4
4dc2908
46c28b0
1b3ae0a
36eef3b
5a4ece4
51f863c
77c819a
3e7c65f
7e4ea48
a893d28
d0e0b8c
a5c3076
6f09279
c5ffbfe
987c35a
c1df675
eda6a0d
50788cd
25f9c37
c31dccb
3e3da34
5c8235f
d56fcd8
caf16a9
2c3495a
4ba5c62
65eca57
0248c92
749216c
2db9f2e
5c2923e
3301f7d
6a3a747
c02d721
8ec44cb
dd1c994
a3251c8
91ef8f6
4184cc9
598707c
aa17a19
12a6b5f
63e317b
b3543e7
0e1db58
903f4a9
4639429
83068d7
0a9b77b
9b86efe
9396758
b888d84
6086be4
049eb4b
1471622
2e815be
473ad60
b88d649
31c615a
b57ae44
f7aebcd
40f2601
f3dbb6f
bac28d0
3cf3f0d
2f4fb1f
c7250f6
602c9e8
a6dae5c
9e130e0
c4a81a7
4677cbd
d002ce3
08cbeb6
7119740
83eb947
0ba9fb2
8fe92b9
0866a5c
22c6a32
a578122
b05f085
b585fbc
b370000
9c01894
41c3344
d5b3378
13db273
ae1234f
f23d36e
111fdee
e7631bb
30bbb0b
9abcacb
3b12c25
de44fec
5ec5b02
d2e6890
fa4810f
b07b782
0d27a4b
15467f1
9ba7e91
9e2daee
ad82225
2a7a2cf
fd9cf77
742a035
9e2cded
f5620c2
f86e72b
6c6dda6
bca6772
a63c6f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ | |
| #include "AOTEffectsCheck.h" | ||
| #include "AOTEffectsOfProtocols.h" | ||
| #include "AOTFlowCoroutines.h" | ||
| #include "AOTFlowResults.h" | ||
| #include "AOTFlowUnseenCode.h" | ||
| #include "AOTFunctionAnalyses.h" | ||
| #include "AOTFunctionEditing.h" | ||
| #include "AOTFuseGeneratorLoops.h" | ||
|
|
@@ -30,6 +32,7 @@ | |
| #include "AOTValueSubstitution.h" | ||
| #include "AOTVector.h" | ||
| #include <algorithm> | ||
| #include <array> | ||
| #include <initializer_list> | ||
| #include <optional> | ||
| #include <ranges> | ||
|
|
@@ -91,6 +94,22 @@ ValueID appendAsyncIteratorStep(IRBuilder& builder, ValueID iterator, ValueID se | |
| return builder.append(Opcode::StepAsyncGenerator, { iterator, sent, driver }); | ||
| } | ||
|
|
||
| Value makeIteratorResultObject(const Program& program, ValueID value, ValueID done, AllocationSiteID site) | ||
| { | ||
| LayoutID layoutID = program.layoutOf(BuiltinLayouts::IteratorResult); | ||
| const Layout& layout = program.at(layoutID); | ||
| std::optional<uint32_t> valueField = layout.findField(program.keyOf(BuiltinNames::value)); | ||
| std::optional<uint32_t> doneField = layout.findField(program.keyOf(BuiltinNames::done)); | ||
| RELEASE_ASSERT(valueField && doneField); | ||
|
|
||
| Value object = IRBuilder::make(Opcode::NewObject); | ||
| object.setLayout(layoutID); | ||
| object.setFields(std::array { *valueField, *doneField }); | ||
| object.setValuesOperands(std::array { value, done }); | ||
| object.setAllocationSite(site); | ||
| return object; | ||
| } | ||
|
|
||
| // A ramp takes over the id and the callers of its source, so what is known about those callers carries over. | ||
| static constexpr OptionSet<FunctionCapabilities> capabilitiesRampsInherit { | ||
| FunctionCapabilities::MayChangeSignature, | ||
|
|
@@ -696,8 +715,15 @@ static void retypeCallers(ProgramEditor& editor, BuildContext& build, const Vect | |
| }); | ||
| } | ||
|
|
||
| static bool loadsModulesByName(const FlowResults& flow) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 nit (optional): a maintainer adding sources to aot/ can get a bundle that no longer compiles, because this static reuses a name that another source hands on by itself. The new static Why this was flaggedThe root CLAUDE.md rule for aot/ says whatever a source declares for itself at namespace scope has a name nothing else in aot/ has, because sources are unified 16 (8 in Bun) to a bundle in Sources.txt order and adding a source shifts every bundle after it. AOTCoroutines.cpp:718 declares Verification: AOTCoroutines.cpp:718 adds |
||
| { | ||
| return !flow.hasResults() || std::ranges::any_of(flow.unseenCode(), [](const UnseenCodeRecord& record) { | ||
| return loadsModulesByName(record.cause); | ||
| }); | ||
| } | ||
|
|
||
| // A require() evaluates the ES module that it names and what that module imports. | ||
| static void markModulesEvaluatedByRequire(ProgramEditor& editor, const ParallelLoops& loops) | ||
| static void markModulesEvaluatedByRequire(ProgramEditor& editor, const FlowResults& flow, const ParallelLoops& loops) | ||
| { | ||
| const Program& program = editor.program(); | ||
| bool someModuleAwaits = std::ranges::any_of(program.modules, [](const Module& module) { | ||
|
|
@@ -720,6 +746,11 @@ static void markModulesEvaluatedByRequire(ProgramEditor& editor, const ParallelL | |
| work.append(request.target.index()); | ||
| } | ||
| } | ||
| // A name computed at run time finds an entry point. A program with one entry point has begun it before any require() runs. | ||
| if (program.entries.size() > 1 && loadsModulesByName(flow)) { | ||
| for (const Entry& entry : program.entries) | ||
| work.append(entry.module.index()); | ||
| } | ||
| BitVector seen(program.modules.size()); | ||
| while (!work.isEmpty()) { | ||
| uint32_t index = work.takeLast(); | ||
|
|
@@ -740,7 +771,7 @@ void splitCoroutines(ProgramEditor& editor, const OptimizationInputs& inputs, co | |
| }); | ||
| if (coroutines.isEmpty()) | ||
| return; | ||
| markModulesEvaluatedByRequire(editor, build.parallelLoops()); | ||
| markModulesEvaluatedByRequire(editor, inputs.flow, build.parallelLoops()); | ||
| Vector<CoroutinePlan> plans(coroutines.size(), [&](size_t index) { | ||
| CoroutinePlan plan; | ||
| plan.record.kind = *coroutineKindOf(program.functions[coroutines[index]]); | ||
|
|
@@ -1329,11 +1360,8 @@ static void expandAwaitOfGeneratorStep(FunctionPassContext& context, IRBuilder& | |
| if (!site.resultReads) { | ||
| builder.endExpansion(expansion, convertTo(builder, stepped, DeclaredType { .type = type })); | ||
|
|
||
| // What CreateIterResultObject makes: an ordinary object with the two as its own data properties, in this order. | ||
| builder.moveToEndOf(blocks.notTaken); | ||
| ValueID object = builder.append(Opcode::NewDictionary, { }); | ||
| appendWithKey(Opcode::PropDefine, { object, stepped }, valueKey); | ||
| appendWithKey(Opcode::PropDefine, { object, appendStatusTest(AsyncGeneratorStepStatus::Completed) }, doneKey); | ||
| ValueID object = builder.append(makeIteratorResultObject(context.program, stepped, appendStatusTest(AsyncGeneratorStepStatus::Completed))); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Programs that await Why this was flaggedA compiled program does Verification: AOTCoroutines.cpp:1360-1364 emits a |
||
| builder.endExpansion(expansion, convertTo(builder, object, DeclaredType { .type = type })); | ||
| return; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔎 Supported by static analysis
Script executed:
Repository: oven-sh/WebKit
Length of output: 7884
Script executed:
Repository: oven-sh/WebKit
Length of output: 36845
Keep verifier state growth within the visit budget.
findUsesOfUnspecifiedValuesdefaults to 4,194,304 visits. The new merge logic can retain up to eight states for one block and marked-value set.walkAllconsumes one visit for each queued state and setswasEveryPathFollowedfalse when the budget is exhausted. The verifier then fails when the walk gives up without finding a use. A function with enough such joins can therefore fail verification even when its paths are valid. Ensure the state expansion cannot exhaust this budget, or adjust the verifier’s budget or give-up handling.