Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebGPU] built-in arrayLength assumes all buffers require an arrayLength, when it should not #18718

Conversation

mwyrzykowski
Copy link
Contributor

@mwyrzykowski mwyrzykowski commented Oct 5, 2023

d7c9c0b

[WebGPU] built-in arrayLength assumes all buffers require an arrayLength, when it should not
https://bugs.webkit.org/show_bug.cgi?id=262728
<radar://116539036>

Reviewed by Dan Glastonbury.

In 268884@main, a buffer length for all buffers was generated. This is not
really needed.

Instead, generate a buffer length only when it is needed in the shader.

* Source/WebGPU/WGSL/GlobalVariableRewriter.cpp:
(WGSL::RewriteGlobalVariables::determineUsedGlobals):
* Source/WebGPU/WGSL/WGSL.h:
* Source/WebGPU/WebGPU/BindGroup.mm:
(WebGPU::Device::createBindGroup):
* Source/WebGPU/WebGPU/BindGroupLayout.mm:
(WebGPU::createArgumentDescriptor):
(WebGPU::Device::createBindGroupLayout):
(WebGPU::BindGroupLayout::bufferSizeIndexForEntryIndex const):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::addPipelineLayouts):
* Source/WebGPU/WebGPU/WebGPUExt.h:

Canonical link: https://commits.webkit.org/268969@main

71a8efc

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt   πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Oct 5, 2023
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label Oct 5, 2023
Copy link
Contributor

@djg djg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@mwyrzykowski mwyrzykowski added the merge-queue Applied to send a pull request to merge-queue label Oct 6, 2023
…gth, when it should not

https://bugs.webkit.org/show_bug.cgi?id=262728
<radar://116539036>

Reviewed by Dan Glastonbury.

In 268884@main, a buffer length for all buffers was generated. This is not
really needed.

Instead, generate a buffer length only when it is needed in the shader.

* Source/WebGPU/WGSL/GlobalVariableRewriter.cpp:
(WGSL::RewriteGlobalVariables::determineUsedGlobals):
* Source/WebGPU/WGSL/WGSL.h:
* Source/WebGPU/WebGPU/BindGroup.mm:
(WebGPU::Device::createBindGroup):
* Source/WebGPU/WebGPU/BindGroupLayout.mm:
(WebGPU::createArgumentDescriptor):
(WebGPU::Device::createBindGroupLayout):
(WebGPU::BindGroupLayout::bufferSizeIndexForEntryIndex const):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::addPipelineLayouts):
* Source/WebGPU/WebGPU/WebGPUExt.h:

Canonical link: https://commits.webkit.org/268969@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-built-in-arrayLength-assumes-all-buffers-require-an-arrayLength-when-it-should-not branch from 71a8efc to d7c9c0b Compare October 6, 2023 04:55
@webkit-commit-queue
Copy link
Collaborator

Committed 268969@main (d7c9c0b): https://commits.webkit.org/268969@main

Reviewed commits have been landed. Closing PR #18718 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit d7c9c0b into WebKit:main Oct 6, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebGPU For bugs in WebGPU
Projects
None yet
4 participants