Bug 198186 – [WHLSL] Standard library is too big to directly include in WebCore

• Component: WebGPU
• Status: RESOLVED FIXED

[Myles C. Maxfield]

The raw text of the standard library is multiple megabytes, and we can’t increase the binary of WebCore by multiple megabytes.

[Radar WebKit Bug Importer]

<rdar://problem/51288898>

[Saam Barati]

Ideas Myles has suggested:
- Gzip the stdlib text into the binary
- To combat large compile times, prune the call graph
- Remove some stdlib from text, and build it in as some kind of an intrinsic in the compiler itself

Some of these tasks should be independent of each other

[Myles C. Maxfield]

Created attachment 372733 [details]
WIP

[Myles C. Maxfield]

Created attachment 372942 [details]
WIP

[Myles C. Maxfield]

Created attachment 372998 [details]
Almost full standard library

[Myles C. Maxfield]

Pieces missing:

step()
sign()
faceforward()
Some of the texturing functions
RWTextures
Loading from non-base mipmap

[Myles C. Maxfield]

Created attachment 373090 [details]
Too slow

[Myles C. Maxfield]

Created attachment 373143 [details]
Passes tests but is still too slow

[Myles C. Maxfield]

4.07 s   98.2%	0 s	 	                                                        WebCore::WHLSL::prepare(WTF::String&, WebCore::WHLSL::ComputePipelineDescriptor&)
2.46 s   59.3%	1.00 ms	 	                                                         WebCore::WHLSL::prepareShared(WTF::String&)
1.20 s   28.8%	0 s	 	                                                          WebCore::WHLSL::check(WebCore::WHLSL::Program&)
389.00 ms    9.3%	0 s	 	                                                          WebCore::WHLSL::Visitor::visit(WebCore::WHLSL::Program&)
286.00 ms    6.9%	0 s	 	                                                           WebCore::WHLSL::PropertyResolver::visit(WebCore::WHLSL::AST::FunctionDefinition&)
67.00 ms    1.6%	0 s	 	                                                           WebCore::WHLSL::RecursionChecker::visit(WebCore::WHLSL::AST::FunctionDefinition&)
22.00 ms    0.5%	0 s	 	                                                           WebCore::WHLSL::Visitor::visit(WebCore::WHLSL::AST::Block&)
11.00 ms    0.2%	0 s	 	                                                           WebCore::WHLSL::Visitor::visit(WebCore::WHLSL::AST::FunctionDeclaration&)
3.00 ms    0.0%	0 s	 	                                                           WebCore::WHLSL::Visitor::visit(WebCore::WHLSL::AST::FunctionDefinition&)
249.00 ms    6.0%	0 s	 	                                                          WebCore::WHLSL::preserveVariableLifetimes(WebCore::WHLSL::Program&)
226.00 ms    5.4%	0 s	 	                                                          WebCore::WHLSL::Parser::parse(WebCore::WHLSL::Program&, WTF::StringView, WebCore::WHLSL::Parser::Mode)
143.00 ms    3.4%	1.00 ms	 	                                                          WebCore::WHLSL::NameResolver::visit(WebCore::WHLSL::AST::FunctionDefinition&)
83.00 ms    2.0%	0 s	 	                                                          WebCore::WHLSL::autoInitializeVariables(WebCore::WHLSL::Program&)
64.00 ms    1.5%	0 s	 	                                                          WebCore::WHLSL::checkTextureReferences(WebCore::WHLSL::Program&)
44.00 ms    1.0%	0 s	 	                                                          WebCore::WHLSL::NameResolver::visit(WebCore::WHLSL::AST::NativeFunctionDeclaration&)
25.00 ms    0.6%	0 s	 	                                                          WebCore::WHLSL::synthesizeConstructors(WebCore::WHLSL::Program&)
23.00 ms    0.5%	0 s	 	                                                          WebCore::WHLSL::synthesizeArrayOperatorLength(WebCore::WHLSL::Program&)
8.00 ms    0.1%	0 s	 	                                                          WebCore::WHLSL::checkDuplicateFunctions(WebCore::WHLSL::Program const&)
7.00 ms    0.1%	0 s	 	                                                          WebCore::WHLSL::checkStatementBehavior(WebCore::WHLSL::Program&)
1.00 ms    0.0%	1.00 ms	 	                                                          WebCore::WHLSL::checkFunctionStages(WebCore::WHLSL::Program&)
1.00 ms    0.0%	0 s	 	                                                          WebCore::WHLSL::resolveNamesInTypes(WebCore::WHLSL::Program&, WebCore::WHLSL::NameResolver&)
927.00 ms   22.3%	0 s	 	                                                         WebCore::WHLSL::Metal::generateMetalCode(WebCore::WHLSL::Program&, WebCore::WHLSL::MatchedComputeSemantics&&, WTF::Vector<WebCore::WHLSL::BindGroup, 0ul, WTF::CrashOnOverflow, 16ul>&)
923.00 ms   22.2%	0 s	 	                                                          WebCore::WHLSL::Metal::TypeNamer::metalTypes()
826.00 ms   19.9%	0 s	 	                                                           WebCore::WHLSL::Visitor::visit(WebCore::WHLSL::Program&)
94.00 ms    2.2%	0 s	 	                                                           WebCore::WHLSL::Metal::TypeNamer::metalTypeDeclarations()
3.00 ms    0.0%	0 s	 	                                                           WebCore::WHLSL::Metal::TypeNamer::metalTypeDefinitions()
4.00 ms    0.0%	1.00 ms	 	                                                          WebCore::WHLSL::Metal::metalFunctions(WebCore::WHLSL::Program&, WebCore::WHLSL::Metal::TypeNamer&, WebCore::WHLSL::MatchedComputeSemantics&&, WTF::Vector<WebCore::WHLSL::BindGroup, 0ul, WTF::CrashOnOverflow, 16ul>&)
599.00 ms   14.4%	3.00 ms	 	                                                         WebCore::WHLSL::Program::~Program()
87.00 ms    2.0%	0 s	 	                                                         WebCore::WHLSL::computeDimensions(WebCore::WHLSL::Program&, WebCore::WHLSL::AST::FunctionDefinition&)

[Myles C. Maxfield]

29% typechecking
7% property resolver
19% building type trie in Metal codegen
14% destroying the program
5% parsing
6% preserveVariableLifetimes

[Myles C. Maxfield]

Created attachment 373164 [details]
Patch

[EWS Watchlist]

Attachment 373164 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 373181 [details]
WIP

[Myles C. Maxfield]

Created attachment 373187 [details]
Patch

[EWS Watchlist]

Attachment 373187 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 373188 [details]
Patch

[EWS Watchlist]

Attachment 373188 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 373188 [details]
Patch

Attachment 373188 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12617205

New failing tests:
http/tests/cache/disk-cache/redirect-chain-limits.html

[EWS Watchlist]

Created attachment 373189 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.5

[Myles C. Maxfield]

Created attachment 373212 [details]
Patch

[Myles C. Maxfield]

Created attachment 373343 [details]
Without standard library changes

[Saam Barati]

Comment on attachment 373343 [details]
Without standard library changes

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

r=me

> Source/WebCore/ChangeLog:29
> +        form /* Functions named xyz */. At build time, a Python script looks for all these comments, and builds a

Is this being done in the spec repo?

> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:44
> +static Vector<LChar> decompressStandardLibrary()

Do we not just have a helper "gunzip" method in WebKit? If not, maybe we can add a "gunzip" helper in WTF?

> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:112
> +        HashSet<String> result;
> +        std::swap(result, m_functionNames);
> +        return result;

style nit:
Why not: "return WTFMove(m_functionNames)"?

> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:151
> +    functionNames.add("operator cast"_str);
> +    functionNames.add("ddx"_str);
> +    functionNames.add("ddy"_str);
> +    functionNames.add("DeviceMemoryBarrierWithGroupSync"_str);
> +    functionNames.add("GroupMemoryBarrierWithGroupSync"_str);
> +    functionNames.add("AllMemoryBarrierWithGroupSync"_str);

This means we always parse these? Why is that needed? Maybe a comment?

> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:166
> +        functionNames.clear();

style nit: this isn't necessary since you're assigning to it below.

> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:177
> +}
> +
> +}

style nit: add namespace ending comments

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLBuildStandardLibraryFunctionMap.py:87
> +    outfile.write("    result.add(\"" + previousName + "\"_str, SubstringLocation { " + str(previous) + ", " + str(match.start()) + " });\n")
> +    previous = match.start()
> +    previousName = match.group(1)
> +outfile.write("    result.add(\"" + previousName + "\"_str, SubstringLocation { " + str(previous) + ", " + str(len(contents)) + " });\n")

Can you open a bug and add a FIXME here about just computing the hash table at compile time in the future? Kinda like we do with the static property hash tables in JSC. We could probably just reuse that code.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:420
> +        || functionDefinition.name() == "operator>>")

👍🏼

> LayoutTests/webgpu/whlsl-matrix.html:39
> +    foo = foo + float2x3(float3(1.0, 1.0, 1.0), float3(1.0, 1.0, 1.0));
>      if (!allEqual(foo, 1))
>          return;
>  
> -    foo = foo * 8.5;
> +    foo = foo * float2x3(float3(8.5, 8.5, 8.5), float3(8.5, 8.5, 8.5));
>      if (!allEqual(foo, 8.5))
>          return;
>  
> -    foo = foo - 7.5;
> +    foo = foo - float2x3(float3(7.5, 7.5, 7.5), float3(7.5, 7.5, 7.5));
>      if (!allEqual(foo, 1))
>          return;
>  
> -    foo = foo + 1.0;
> +    foo = foo + float2x3(float3(1.0, 1.0, 1.0), float3(1.0, 1.0, 1.0));

Why? Isn't this part of the stdlib to do math with a single parameter against all matrix elements?

[Myles C. Maxfield]

Comment on attachment 373343 [details]
Without standard library changes

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

>> Source/WebCore/ChangeLog:29
>> +        form /* Functions named xyz */. At build time, a Python script looks for all these comments, and builds a
> 
> Is this being done in the spec repo?

Nope. I think it's okay if we don't impose the rigorous structure we require in WebKit on the GitHub spec repository.

>> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:112
>> +        return result;
> 
> style nit:
> Why not: "return WTFMove(m_functionNames)"?

Even after running this function, m_functionNames needs to be valid (and empty). If we just moved from it, we would then be using a moved-from object, which is undefined.

>> LayoutTests/webgpu/whlsl-matrix.html:39
>> +    foo = foo + float2x3(float3(1.0, 1.0, 1.0), float3(1.0, 1.0, 1.0));
> 
> Why? Isn't this part of the stdlib to do math with a single parameter against all matrix elements?

Nope! We should add it to the spec repo first, though.

[Saam Barati]

Comment on attachment 373343 [details]
Without standard library changes

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

>>> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:112
>>> +        return result;
>> 
>> style nit:
>> Why not: "return WTFMove(m_functionNames)"?
> 
> Even after running this function, m_functionNames needs to be valid (and empty). If we just moved from it, we would then be using a moved-from object, which is undefined.

FYI, that's not quite right. Moving from things defined in the C++ stdlib is undefined. Moving from things where move operator is defined is not undefined.

[Saam Barati]

Comment on attachment 373343 [details]
Without standard library changes

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

>>>> Source/WebCore/Modules/webgpu/WHLSL/Cocoa/WHLSLStandardLibraryUtilities.cpp:112
>>>> +        return result;
>>> 
>>> style nit:
>>> Why not: "return WTFMove(m_functionNames)"?
>> 
>> Even after running this function, m_functionNames needs to be valid (and empty). If we just moved from it, we would then be using a moved-from object, which is undefined.
> 
> FYI, that's not quite right. Moving from things defined in the C++ stdlib is undefined. Moving from things where move operator is defined is not undefined.

Anyways, if it wasn't obvious, I think it's ok to keep it as you have it defined now.

[Myles C. Maxfield]

Created attachment 373392 [details]
Almost ready for committing

[Myles C. Maxfield]

Committed r247115: <https://trac.webkit.org/changeset/247115>

[Alexey Proskuryakov]

Making each WTF client explicitly link to libcompression doesn't seem great.

[Ryan Haddad]

Reverted r247115 for reason:

Breaks lldbWebKitTester (and by extension, test-webkitpy)

Committed r247164: <https://trac.webkit.org/changeset/247164>

[Ryan Haddad]

*** Bug 199507 has been marked as a duplicate of this bug. ***

[Myles C. Maxfield]

Committed r247174: <https://trac.webkit.org/changeset/247174>