Bug 236697

Summary: [WebGPU] Integrate Metal-cpp
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: WebGPUAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dino, emw, ews-watchlist, glenn, jbedard, rmorisset, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
dino: review+
Patch for committing
ews-feeder: commit-queue-
Patch for committing
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch for committing
ews-feeder: commit-queue-
Rebased none

Myles C. Maxfield
Reported 2022-02-16 01:57:45 PST
[WebGPU] Integrate Metal-cpp
Attachments
Patch (1.19 MB, patch)
2022-02-16 02:00 PST, Myles C. Maxfield
no flags
Patch (960.97 KB, patch)
2022-02-16 18:26 PST, Myles C. Maxfield
no flags
Patch (960.88 KB, patch)
2022-02-16 20:15 PST, Myles C. Maxfield
dino: review+
Patch for committing (959.61 KB, patch)
2022-02-17 16:54 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Patch for committing (959.61 KB, patch)
2022-02-17 17:00 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Patch (959.56 KB, patch)
2022-02-17 17:50 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Patch for committing (959.73 KB, patch)
2022-02-17 20:06 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Rebased (959.67 KB, patch)
2022-05-02 00:18 PDT, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2022-02-16 02:00:02 PST
Myles C. Maxfield
Comment 2 2022-02-16 18:26:24 PST
Myles C. Maxfield
Comment 3 2022-02-16 20:15:43 PST
Dean Jackson
Comment 4 2022-02-17 04:48:01 PST
Comment on attachment 452297 [details] Patch rs=me
Myles C. Maxfield
Comment 5 2022-02-17 16:54:46 PST
Created attachment 452446 [details] Patch for committing
Myles C. Maxfield
Comment 6 2022-02-17 17:00:41 PST
Created attachment 452447 [details] Patch for committing
Myles C. Maxfield
Comment 7 2022-02-17 17:50:05 PST
Myles C. Maxfield
Comment 8 2022-02-17 18:28:50 PST
Oh. Metal-cpp isn't supported on armv7k. WebKit does support armv7k, though.
Myles C. Maxfield
Comment 9 2022-02-17 20:06:43 PST
Created attachment 452475 [details] Patch for committing
Myles C. Maxfield
Comment 10 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.
Radar WebKit Bug Importer
Comment 11 2022-02-23 01:58:23 PST
Myles C. Maxfield
Comment 12 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.
Myles C. Maxfield
Comment 13 2022-05-02 00:18:33 PDT
Alexey Proskuryakov
Comment 14 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.
Elliott Williams
Comment 15 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.
Myles C. Maxfield
Comment 16 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
Myles C. Maxfield
Comment 17 2022-05-20 16:35:36 PDT
*** Bug 240746 has been marked as a duplicate of this bug. ***
Myles C. Maxfield
Comment 18 2022-05-20 18:05:04 PDT
Note You need to log in before you can comment on or make changes to this bug.