[webkit-reviews] review granted: [Bug 200512] [WHLSL] Metal code generation takes a long time uniquing UnnamedTypes : [Attachment 375728] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 200512: [WHLSL] Metal code generation takes a long time uniquing
UnnamedTypes
https://bugs.webkit.org/show_bug.cgi?id=200512

Attachment 375728: Patch

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




--- Comment #3 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 375728
  --> https://bugs.webkit.org/attachment.cgi?id=375728
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