Bug 216713 - [WebGPU] Add ENABLE(WHLSL_COMPILER) guard
Summary: [WebGPU] Add ENABLE(WHLSL_COMPILER) guard
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks: 216712
  Show dependency tree / graph
 
Reported: 2020-09-18 12:18 PDT by Don Olmstead
Modified: 2020-09-18 15:55 PDT (History)
8 users (show)

See Also:

Attachments
Patch (120.68 KB, patch)
2020-09-18 12:36 PDT, Don Olmstead 		mmaxfield: review+
Details | Formatted Diff | Diff
Patch (120.56 KB, patch)
2020-09-18 13:50 PDT, Don Olmstead 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2020-09-18 12:18:53 PDT 
The WHLSL compiler is Metal specific.
Comment 1 Don Olmstead 2020-09-18 12:36:42 PDT 
Created attachment 409164 [details]
Patch
Comment 2 Myles C. Maxfield 2020-09-18 13:27:02 PDT 
Comment on attachment 409164 [details]
Patch

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

> Source/WTF/wtf/PlatformEnableCocoa.h:564
> +#define ENABLE_WEBGPU 0

This shouldn't be unnecessary. The ENABLE() macro yields false if the token isn't defined.
Comment 3 Myles C. Maxfield 2020-09-18 13:28:01 PDT 
Comment on attachment 409164 [details]
Patch

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

>> Source/WTF/wtf/PlatformEnableCocoa.h:564
>> +#define ENABLE_WEBGPU 0
> 
> This shouldn't be unnecessary. The ENABLE() macro yields false if the token isn't defined.

This shouldn't be unnecessary. The ENABLE() macro yields false if the token isn't defined.

> Source/WTF/wtf/PlatformEnableCocoa.h:568
> +#define ENABLE_WHLSL_COMPILER ENABLE_WEBGPU

Why can't we just put this #define in the above block, and #define it to 1? I don't think we have precedence in these PlatformEnable files for defining things to be equal to other things.
Comment 4 Myles C. Maxfield 2020-09-18 13:28:59 PDT 
s/precedence/precedent/
Comment 5 Darin Adler 2020-09-18 13:46:02 PDT 
Comment on attachment 409164 [details]
Patch

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

>> Source/WTF/wtf/PlatformEnableCocoa.h:568
>> +#define ENABLE_WHLSL_COMPILER ENABLE_WEBGPU
> 
> Why can't we just put this #define in the above block, and #define it to 1? I don't think we have precedence in these PlatformEnable files for defining things to be equal to other things.

I wouldn’t combine the blocks. But I would write it like this:

#if !defined(ENABLE_WHLSL_COMPILER) && ENABLE(WEBGPU)
#define ENABLE_WHLSL_COMPILER 1
#endif
Comment 6 Darin Adler 2020-09-18 13:46:52 PDT 
Comment on attachment 409164 [details]
Patch

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

>>> Source/WTF/wtf/PlatformEnableCocoa.h:568
>>> +#define ENABLE_WHLSL_COMPILER ENABLE_WEBGPU
>> 
>> Why can't we just put this #define in the above block, and #define it to 1? I don't think we have precedence in these PlatformEnable files for defining things to be equal to other things.
> 
> I wouldn’t combine the blocks. But I would write it like this:
> 
> #if !defined(ENABLE_WHLSL_COMPILER) && ENABLE(WEBGPU)
> #define ENABLE_WHLSL_COMPILER 1
> #endif

Or alternatively we could always check both in the code, and have ENABLE_WHLSL_COMPILER be a no-op when ENABLE(WEBGPU) is not set.
Comment 7 Don Olmstead 2020-09-18 13:50:48 PDT 
Created attachment 409169 [details]
Patch
Comment 8 EWS 2020-09-18 15:54:33 PDT 
Committed r267288: <https://trac.webkit.org/changeset/267288>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409169 [details].
Comment 9 Radar WebKit Bug Importer 2020-09-18 15:55:17 PDT 
<rdar://problem/69180169>