Bug 193877 – [WHLSL] Pack and unpack data at entry points and exit points

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[WHLSL] Pack and unpack data at entry points and exit points

[Myles C. Maxfield]

Created attachment 360258 [details]
WIP

[EWS Watchlist]

Attachment 360258 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 360298 [details]
Patch

[EWS Watchlist]

Attachment 360298 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 360319 [details]
Patch

[EWS Watchlist]

Attachment 360319 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Created attachment 363689 [details]
WIP

[Myles C. Maxfield]

Created attachment 363790 [details]
Patch

[Dean Jackson]

Comment on attachment 363790 [details]
Patch

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

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp:107
> +        namedBindGroup.argumentBufferIndex = argumentBufferIndex++;

Why can't you use i here? (or rename i to be argumentBufferIndex)

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp:113
> +            namedBinding.index = index++;

Why can't you use j here?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp:407
> +    m_namedOutputs.reserveInitialCapacity(m_entryPointItems.outputs.size());
> +    for (size_t i = 0; i < m_entryPointItems.outputs.size(); ++i) {
> +        NamedOutput namedOutput;
> +        namedOutput.elementName = m_typeNamer.generateNextStructureElementName();
> +        m_namedOutputs.uncheckedAppend(WTFMove(namedOutput));
> +    }

I wonder if you could use .map for these loops. It sets up the initial capacity for you.

m_namedOutputs = m_entryPointItems.output.map([] (...) -> NamedOutput {  .... return namedOutput;  }

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:265
> +    CRASH();

Really? Wouldn't we want to return an invalid program instead?

> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLFunctionWriter.cpp:360
> +    CRASH();

ditto for other instances of CRASH()

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLSemanticMatcher.cpp:97
> +                itemIndices.add(i + 1); // Work around the fact that HashSet's keys are restricted.

Confused--- Can't you use a HashSet with a zero key? You test for max above?

[WebKit Commit Bot]

Comment on attachment 363790 [details]
Patch

Clearing flags on attachment: 363790

Committed r242571: <https://trac.webkit.org/changeset/242571>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48654341>