webgpu-native / webgpu-headers

https://webgpu-native.github.io/webgpu-headers/
BSD 3-Clause "New" or "Revised" License
338 stars 38 forks source link

In/out extensibility of WGPULimits and WGPUInstanceFeatures #260

Open kainino0x opened 6 months ago

kainino0x commented 6 months ago

It should be possible to ask wgpuGetInstanceFeatures for more info by adding structs to the chain. As written out here: https://github.com/webgpu-native/webgpu-headers/issues/199#issuecomment-1701605454 this is not possible because nextInChain is const *.

We can fix this by mirroring what we have already for WGPURequiredLimits/WGPUSupportedLimits:

typedef struct WGPUInstanceFeatures {
    WGPUBool timedWaitAnyEnable;
    size_t timedWaitAnyMaxCount;
} WGPUInstanceFeatures WGPU_STRUCTURE_ATTRIBUTE;

typedef struct WGPUSupportedInstanceFeatures {
    WGPUChainedStructOut * nextInChain;
    WGPUInstanceFeatures features; // out
} WGPUSupportedInstanceFeatures WGPU_STRUCTURE_ATTRIBUTE;

typedef struct WGPUInstanceDescriptor {
    WGPUChainedStruct const * nextInChain;
    WGPUInstanceFeatures features; // in
} WGPUInstanceDescriptor WGPU_STRUCTURE_ATTRIBUTE;

Unfortunately for both this and WGPULimits, we need to duplicate stuff for every extension, like:

typedef struct WGPUDawnExperimentalSubgroupLimits {
    uint32_t minSubgroupSize;
    uint32_t maxSubgroupSize;
} WGPUDawnExperimentalSubgroupLimits WGPU_STRUCTURE_ATTRIBUTE;

// Can be chained in WGPUSupportedLimits
typedef struct WGPUDawnExperimentalSubgroupSupportedLimits {
    WGPUChainedStructOut chain;
    WGPUDawnExperimentalSubgroupLimits limits; // out
} WGPUDawnExperimentalSubgroupSupportedLimits WGPU_STRUCTURE_ATTRIBUTE;

// Can be chained in WGPURequiredLimits
typedef struct WGPUDawnExperimentalSubgroupRequiredLimits {
    WGPUChainedStruct chain;
    WGPUDawnExperimentalSubgroupLimits limits; // in
} WGPUDawnExperimentalSubgroupRequiredLimits WGPU_STRUCTURE_ATTRIBUTE;

(Dawn is actually currently missing an equivalent of the latter. I suspect this is because we're abusing "limits" for what are really adapter properties?)

Note the JS API has a similar "problem" except it doesn't have to deal with extension structs.


A totally different option could be to use an array of structs instead of a chain of structs, kind of a key-value-pair system vaguely like what JS uses. We could use this for both. Example for limits:

struct WGPUExtendedLimitsBase {
    WGPUSType sType;
};

struct WGPUDawnExperimentalSubgroupLimits {
    WGPUExtendedLimitsBase type;
    uint32_t minSubgroupSize;
    uint32_t maxSubgroupSize;
};

// Pass in an array of pointers to the limit structs you want populated
WGPUBool wgpuAdapterGetLimits(WGPUAdapter adapter, WGPULimits * limits,
    size_t extendedLimitStructCount, WGPUExtendedLimitsBase * const * extendedLimitStructs);

struct WGPUDeviceDescriptor {
    // ...
    WGPU_NULLABLE WGPURequiredLimits const * requiredLimits;
    // Pass in an array of pointers to the limit structs containing limits you require.
    size_t requiredExtendedLimitStructCount;
    WGPU_NULLABLE WGPULimitStructBase const * const * requiredExtendedLimitsStructs;
    // ...
};

Or we could go all the way to what JS has with a real key-value interface, which is much simpler to write out - though probably not easier to use, and unfortunately a much more breaking change.

struct WGPULimit {
    WGPULimitName name;
    // Due to JS we are probably unlikely to ever have limits that can't be doubles (though possible with BigInt).
    // Note we no longer need WGPU_LIMIT_*_UNDEFINED because you can just not list the limit.
    double value;
};

WGPUBool wgpuAdapterGetLimits(WGPUAdapter adapter, size_t limitCount, WGPULimit * limits);

struct WGPUDeviceDescriptor {
    // ...
    size_t limitCount;
    WGPULimit const * limits;
    // ...
};

Related to #216. cc @lokokung

kainino0x commented 5 months ago

Jan 25 meeting:

jspanchu commented 1 month ago

Looks like the WGPUInstanceDescriptor in the existing webgpu.h header from this repo doesn't have the extra WGPUInstanceFeatures member. For this reason, when I try to dynamically load libwebgpu_dawn.so and use webgpu.h from this repository, wgpuCreateInstance complains.

Please let me know if a separate issue is required, I'd be happy to create one.

Sample:

// clang -g -O0 -fno-omit-frame-pointer -fsanitize=memory main.c
#include <dlfcn.h>
#include <stdlib.h>

#define WGPU_SKIP_DECLARATIONS
#include "webgpu.h"

int main(int argc, char **argv) {
  void *lib =
      dlopen("./build/build/dawn/src/dawn/native/libwebgpu_dawn.so", RTLD_LAZY);
  WGPUProcCreateInstance wgpuCreateInstance =
      (WGPUProcCreateInstance)dlsym(lib, "wgpuCreateInstance");
  WGPUProcInstanceRelease wgpuInstanceRelease =
      (WGPUProcInstanceRelease)dlsym(lib, "wgpuInstanceRelease");
  WGPUInstance instance;
  WGPUInstanceDescriptor desc = {};
  desc.nextInChain = NULL;
  instance = wgpuCreateInstance(&desc);

  wgpuInstanceRelease(instance);
  return EXIT_SUCCESS;
}

Error log:

Error: Requested timedWaitAnyMaxCount is not supported
    at Initialize (/home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/build/dawn/src/dawn/native/EventManager.cpp:328)

MemorySanitizer:DEADLYSIGNAL
==288867==ERROR: MemorySanitizer: SEGV on unknown address 0x000000000018 (pc 0x7fb5b1bf85ab bp 0x000000000008 sp 0x7ffd3acbea88 T288867)
==288867==The signal is caused by a WRITE memory access.
==288867==Hint: address points to the zero page.
    #0 0x7fb5b1bf85ab in dawn::RefCount::Decrement() (/home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/build/build/dawn/src/dawn/native/libdawn_native.so+0x3b15ab) (BuildId: e44e3168fb67d0868f9f085719d992121dffce0c)
    #1 0x7fb5b1a95c79 in dawn::native::NativeInstanceRelease(WGPUInstanceImpl*) (/home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/build/build/dawn/src/dawn/native/libdawn_native.so+0x24ec79) (BuildId: e44e3168fb67d0868f9f085719d992121dffce0c)
    #2 0x5598efc354aa in main /home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/main2.c:19:3
    #3 0x7fb5b38dfd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #4 0x7fb5b38dfe3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #5 0x5598efbaf2a4 in _start (/home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/a.out+0x1e2a4) (BuildId: 158826293b6a9eca2cb14269b0a790af48c09c8e)

MemorySanitizer can not provide additional info.
SUMMARY: MemorySanitizer: SEGV (/home/local/KHQ/jaswant.panchumarti/dev/webgpu-for-vtk/build/build/dawn/src/dawn/native/libdawn_native.so+0x3b15ab) (BuildId: e44e3168fb67d0868f9f085719d992121dffce0c) in dawn::RefCount::Decrement()
==288867==ABORTING
kainino0x commented 1 month ago

This repo isn't currently in-sync with any implementation. Since you're using Dawn, for now you need to stick with the webgpu.h provided by Dawn.

We're still working through all the decided-but-not-implemented changes, separately in Dawn/wgpu-native/webgpu-headers and they won't be in sync for a while longer.

jspanchu commented 1 month ago

I see, will wait until they are all in sync.