Bug 236697 - [WebGPU] Integrate Metal-cpp
Summary: [WebGPU] Integrate Metal-cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-16 01:57 PST by Myles C. Maxfield
Modified: 2022-05-20 18:05 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.19 MB, patch)
2022-02-16 02:00 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (960.97 KB, patch)
2022-02-16 18:26 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (960.88 KB, patch)
2022-02-16 20:15 PST, Myles C. Maxfield
dino: review+
Details | Formatted Diff | Diff
Patch for committing (959.61 KB, patch)
2022-02-17 16:54 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (959.61 KB, patch)
2022-02-17 17:00 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (959.56 KB, patch)
2022-02-17 17:50 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (959.73 KB, patch)
2022-02-17 20:06 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Rebased (959.67 KB, patch)
2022-05-02 00:18 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2022-02-16 01:57:45 PST
[WebGPU] Integrate Metal-cpp
Comment 1 Myles C. Maxfield 2022-02-16 02:00:02 PST
Created attachment 452163 [details]
Patch
Comment 2 Myles C. Maxfield 2022-02-16 18:26:24 PST
Created attachment 452284 [details]
Patch
Comment 3 Myles C. Maxfield 2022-02-16 20:15:43 PST
Created attachment 452297 [details]
Patch
Comment 4 Dean Jackson 2022-02-17 04:48:01 PST
Comment on attachment 452297 [details]
Patch

rs=me
Comment 5 Myles C. Maxfield 2022-02-17 16:54:46 PST
Created attachment 452446 [details]
Patch for committing
Comment 6 Myles C. Maxfield 2022-02-17 17:00:41 PST
Created attachment 452447 [details]
Patch for committing
Comment 7 Myles C. Maxfield 2022-02-17 17:50:05 PST
Created attachment 452458 [details]
Patch
Comment 8 Myles C. Maxfield 2022-02-17 18:28:50 PST
Oh. Metal-cpp isn't supported on armv7k. WebKit does support armv7k, though.
Comment 9 Myles C. Maxfield 2022-02-17 20:06:43 PST
Created attachment 452475 [details]
Patch for committing
Comment 10 Myles C. Maxfield 2022-02-18 09:27:10 PST
1. TAPI is giving errors about how there aren't actually any symbols for armv7k. I can fix this by adding a dummy symbol.
2. If I add a dummy symbol, I have to have the set of symbols in the headers match the set of symbols in the dylib. This means I have to add __ARM_ARCH checks in the headers.
3. Not only do I have to add checks in the headers, but I have to add the check in _every_ header, because TAPI looks at _all_ the headers, not just the umbrella header. This means I'd have to modify the wgpu shared header, which is something I'd rather not do (e.g. every time it gets updated I'd have to re-apply the local modifications)

So, the fact that I'd need
- To modify the wgpu shared header
- A source file and a dummy header that includes a dummy symbol which is unused
- Custom EXCLUDED_SOURCE_FILE_NAMES Xcode variables, specialized for different architectures
I think is a bridge too far. I don't think I should commit this until there's some better way of doing it.
Comment 11 Radar WebKit Bug Importer 2022-02-23 01:58:23 PST
<rdar://problem/89344870>
Comment 12 Myles C. Maxfield 2022-03-15 22:39:36 PDT
This patch is on hold because WebKit builds on armv7k but Metal-cpp doesn't support 32-bit platforms.
Comment 13 Myles C. Maxfield 2022-05-02 00:18:33 PDT
Created attachment 458665 [details]
Rebased
Comment 14 Alexey Proskuryakov 2022-05-19 10:12:07 PDT
Comment on attachment 458665 [details]
Rebased

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

> Source/WebGPU/WebGPU.xcodeproj/project.pbxproj:1005
> +			shellScript = "mkdir -p ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal\n\ncd WebGPU/metal-cpp\npython3 ./SingleHeader/MakeSingleHeader.py -o ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal/Metal.hpp Foundation/Foundation.hpp QuartzCore/QuartzCore.hpp Metal/Metal.hpp\n";

You can (probably should) use ${SCRIPT_INPUT_FILE_0} and ${SCRIPT_OUTPUT_FILE_0} here.

How long does this script take? It may be useful to skip it for installhdrs and installsrc.
Comment 15 Elliott Williams 2022-05-19 11:13:39 PDT
Comment on attachment 458665 [details]
Rebased

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

> Source/WebGPU/Configurations/WebGPU.xcconfig:96
> +HEADERMAP_INCLUDES_PROJECT_HEADERS = NO;

I believe this setting in meaningless in the new build system. Can you remove it and see if it still builds, now that we have migrated over?

>> Source/WebGPU/WebGPU.xcodeproj/project.pbxproj:1005
>> +			shellScript = "mkdir -p ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal\n\ncd WebGPU/metal-cpp\npython3 ./SingleHeader/MakeSingleHeader.py -o ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal/Metal.hpp Foundation/Foundation.hpp QuartzCore/QuartzCore.hpp Metal/Metal.hpp\n";
> 
> You can (probably should) use ${SCRIPT_INPUT_FILE_0} and ${SCRIPT_OUTPUT_FILE_0} here.
> 
> How long does this script take? It may be useful to skip it for installhdrs and installsrc.

I think it would be better to set USE_RECURSIVE_SCRIPT_INPUTS_IN_SCRIPT_PHASES=YES for this target, and then have an input on the whole `$(SRCROOT)/WebGPU/metal-cpp` directory. Listing every file here is begging for it to get out of sync.

nit: mkdir -p is unnecessary. The build system creates directories for any output path ahead of time.
Comment 16 Myles C. Maxfield 2022-05-20 15:23:33 PDT
Comment on attachment 458665 [details]
Rebased

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

>>> Source/WebGPU/WebGPU.xcodeproj/project.pbxproj:1005
>>> +			shellScript = "mkdir -p ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal\n\ncd WebGPU/metal-cpp\npython3 ./SingleHeader/MakeSingleHeader.py -o ${BUILT_PRODUCTS_DIR}/DerivedSources/WebGPU/metal-cpp/Metal/Metal.hpp Foundation/Foundation.hpp QuartzCore/QuartzCore.hpp Metal/Metal.hpp\n";
>> 
>> You can (probably should) use ${SCRIPT_INPUT_FILE_0} and ${SCRIPT_OUTPUT_FILE_0} here.
>> 
>> How long does this script take? It may be useful to skip it for installhdrs and installsrc.
> 
> I think it would be better to set USE_RECURSIVE_SCRIPT_INPUTS_IN_SCRIPT_PHASES=YES for this target, and then have an input on the whole `$(SRCROOT)/WebGPU/metal-cpp` directory. Listing every file here is begging for it to get out of sync.
> 
> nit: mkdir -p is unnecessary. The build system creates directories for any output path ahead of time.

All good ideas.

The script is pretty much instantaneous:

python3 ./SingleHeader/MakeSingleHeader.py -o ~/tmp/Metal.hpp     0.15s user 0.08s system 76% cpu 0.304 total
Comment 17 Myles C. Maxfield 2022-05-20 16:35:36 PDT
*** Bug 240746 has been marked as a duplicate of this bug. ***
Comment 18 Myles C. Maxfield 2022-05-20 18:05:04 PDT
https://commits.webkit.org/250827@main