[webkit-reviews] review granted: [Bug 200352] [WHLSL] Do simple nullptr check elimination using basic data flow analysis when generating metal code : [Attachment 375341] patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 200352: [WHLSL] Do simple nullptr check elimination using basic data flow
analysis when generating metal code
https://bugs.webkit.org/show_bug.cgi?id=200352

Attachment 375341: patch

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




--- Comment #4 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 375341
  --> https://bugs.webkit.org/attachment.cgi?id=375341
patch

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

> Source/WebCore/ChangeLog:9
> +	   When doing metal code generation, we frequently know whether
something
> +	   is null or not. This patch does a basic propagation of this
information

Sorry for being dumb, but do you think you could provide an example?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:196
> +    std::pair<String, Nullability> takeLastValueAndNullability()

Adding this stuff into a pair seems yucky. I expect in the future we'll be
adding more and more data here. Do you think it would be better design to put
this into a struct? Or just use StackItem directly?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:668
> +	   auto lValue = takeLastLeftValue().first;

Again, a struct type would be a better design here. Unclear what "first" means.