Bug 200512 - [WHLSL] Metal code generation takes a long time uniquing UnnamedTypes
Summary: [WHLSL] Metal code generation takes a long time uniquing UnnamedTypes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-07 11:49 PDT by Sam Weinig
Modified: 2019-08-07 16:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (46.06 KB, patch)
2019-08-07 11:59 PDT, Sam Weinig
sbarati: review+
sam: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2019-08-07 11:49:36 PDT
[WHLSL] Metal code generation takes a long time uniquing UnnamedTypes
Comment 1 Sam Weinig 2019-08-07 11:59:36 PDT
Created attachment 375728 [details]
Patch
Comment 2 Sam Weinig 2019-08-07 14:40:09 PDT
This is about a ~1ms improvement (10%) on the metal codegen phase.
Comment 3 Saam Barati 2019-08-07 16:27:17 PDT
Comment on attachment 375728 [details]
Patch

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

Nice. r=me

> Source/WebCore/ChangeLog:13
> +        the UnnamedType subtree, we should probably do it for the entire AST in a future
> +        change.

Yeah I think this will be a huge speedup. Walking the entire AST on compute_boids is like ~1ms. I think it will be much faster once we remove virtual function calls for AST node type checks.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLPrepare.cpp:66
> +static constexpr bool dumpPhaseTimes = true;

please revert

> Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h:70
> +    Kind kind() const { return m_kind; }
> +    bool isTypeReference() const { return m_kind == Kind::TypeReference; }
> +    bool isPointerType() const { return m_kind == Kind::PointerType; }
> +    bool isArrayReferenceType() const { return m_kind == Kind::ArrayReferenceType; }
> +    bool isArrayType() const { return m_kind == Kind::ArrayType; }
> +    bool isReferenceType() const { return isPointerType() || isArrayReferenceType(); }

nice

> Source/WebCore/Modules/webgpu/WHLSL/AST/WHLSLUnnamedType.h:76
> +    unsigned hash() const;
> +    bool operator==(const UnnamedType&) const;

It is worth defining these in an inline file somewhere since they're super small functions? Maybe we could do this in a follow up
Comment 4 Saam Barati 2019-08-07 16:28:42 PDT
Comment on attachment 375728 [details]
Patch

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

>> Source/WebCore/ChangeLog:13
>> +        change.
> 
> Yeah I think this will be a huge speedup. Walking the entire AST on compute_boids is like ~1ms. I think it will be much faster once we remove virtual function calls for AST node type checks.

We should also consider uniquing types right after name resolution runs. Then we make type checking faster too.
Comment 5 Sam Weinig 2019-08-07 16:47:51 PDT
Committed r248395: <https://trac.webkit.org/changeset/248395>
Comment 6 Radar WebKit Bug Importer 2019-08-07 16:48:18 PDT
<rdar://problem/54056881>