[webkit-reviews] review granted: [Bug 201185] [WHLSL] Inline typedef'd types during Metal code generation to simplify generated code while also making it easier to read : [Attachment 377373] 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 201185: [WHLSL] Inline typedef'd types during Metal code generation to
simplify generated code while also making it easier to read
https://bugs.webkit.org/show_bug.cgi?id=201185

Attachment 377373: Patch

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




--- Comment #5 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 377373
  --> https://bugs.webkit.org/attachment.cgi?id=377373
Patch

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

This is very nice. Does it have an effect on performance? If so, maybe we
should cache the resulting strings?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:-320
> -    auto iterator = m_unnamedTypeMapping.find(UnnamedTypeKey { unnamedType
});

Maybe it’s worth keeping this so we don’t repeatedly make the same strings and
just mapping it to MangledOrNativeTypeName now?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:276
> +	   auto& parent = typeReference.resolvedType();

This is a weird name for this variable. I’m not sure it needs to exist. I’d
just omit giving this expression a variable

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:281
> +	   auto& parent = pointerType.elementType();

Ditto

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:287
> +	   auto& parent = arrayType.type();

Ditto