| Summary: | [WebGPU] Integrate Metal-cpp | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||||||||||||
| Component: | WebGPU | Assignee: | 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
Myles C. Maxfield
2022-02-16 01:57:45 PST
Created attachment 452163 [details]
Patch
Created attachment 452284 [details]
Patch
Created attachment 452297 [details]
Patch
Comment on attachment 452297 [details]
Patch
rs=me
Created attachment 452446 [details]
Patch for committing
Created attachment 452447 [details]
Patch for committing
Created attachment 452458 [details]
Patch
Oh. Metal-cpp isn't supported on armv7k. WebKit does support armv7k, though. Created attachment 452475 [details]
Patch for committing
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. This patch is on hold because WebKit builds on armv7k but Metal-cpp doesn't support 32-bit platforms. Created attachment 458665 [details]
Rebased
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 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 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 *** Bug 240746 has been marked as a duplicate of this bug. *** |