Bug 200512

Summary: [WHLSL] Metal code generation takes a long time uniquing UnnamedTypes
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch saam: review+

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>