Bug 200463 - [WHLSL] Reduce the number of variables that make it into the global struct by skipping stdlib functions and internal uses of MakePointerExpression/MakeArrayReference
Summary: [WHLSL] Reduce the number of variables that make it into the global struct by...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks: 199932
  Show dependency treegraph
 
Reported: 2019-08-05 18:32 PDT by Saam Barati
Modified: 2019-08-06 10:38 PDT (History)
9 users (show)

See Also:


Attachments
WIP (32.68 KB, patch)
2019-08-05 19:04 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (47.68 KB, patch)
2019-08-05 22:36 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-08-05 18:32:18 PDT
....
Comment 1 Saam Barati 2019-08-05 19:04:15 PDT
Created attachment 375593 [details]
WIP

It's good to go, I just need to make it into landing shape. It's 20-30ms faster on compute_boids
Comment 2 Saam Barati 2019-08-05 22:36:18 PDT
Created attachment 375602 [details]
patch
Comment 3 Myles C. Maxfield 2019-08-05 23:19:58 PDT
Comment on attachment 375602 [details]
patch

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

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:128
> +    return AST::NativeFunctionDeclaration(AST::FunctionDeclaration(location, AST::AttributeBlock(), WTF::nullopt, WTFMove(returnType), String("operator&[]", String::ConstructFromLiteral), WTFMove(parameters), nullptr, isOperator, ParsingMode::StandardLibrary));

Wouldn't all native functions be marked as StandardLibrary?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:62
> +auto Parser::parse(Program& program, StringView stringView, ParsingMode mode) -> Expected<void, Error>

is "ParsingMode" really the best name for this? It doesn't have much to do with parsing.
Comment 4 Myles C. Maxfield 2019-08-05 23:20:36 PDT
Comment on attachment 375602 [details]
patch

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

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:62
>> +auto Parser::parse(Program& program, StringView stringView, ParsingMode mode) -> Expected<void, Error>
> 
> is "ParsingMode" really the best name for this? It doesn't have much to do with parsing.

Oh, I misunderstood this. Disregard.
Comment 5 Saam Barati 2019-08-06 10:27:33 PDT
Comment on attachment 375602 [details]
patch

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

>> Source/WebCore/Modules/webgpu/WHLSL/WHLSLChecker.cpp:128
>> +    return AST::NativeFunctionDeclaration(AST::FunctionDeclaration(location, AST::AttributeBlock(), WTF::nullopt, WTFMove(returnType), String("operator&[]", String::ConstructFromLiteral), WTFMove(parameters), nullptr, isOperator, ParsingMode::StandardLibrary));
> 
> Wouldn't all native functions be marked as StandardLibrary?

Yeah they are. But right now, our constructor for NativeFunctionDeclaration takes a FunctionDeclaration as an rvalue. It's kinda weird. We should do a refactoring where we just give it the real constructor parameters directly. When we do that, we can omit ParsingMode and it can directly pass StandardLibrary to the parent constructor.
Comment 6 WebKit Commit Bot 2019-08-06 10:37:15 PDT
Comment on attachment 375602 [details]
patch

Clearing flags on attachment: 375602

Committed r248303: <https://trac.webkit.org/changeset/248303>
Comment 7 WebKit Commit Bot 2019-08-06 10:37:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2019-08-06 10:38:19 PDT
<rdar://problem/53992512>