[UR][Loader] Fix handling of native handles

[alexbatashev]

Native handles are created by adapters and thus are inheritently backend-specific. Loader can not assume anything about these handles, as even nullptr may be a valid value for such a handle.

This patch changes two things about native handles:

1. Native handles are no longer wrapped in UR objects
2. Dispatch table is extracted from any other argument of the API function

The above is true for all interop APIs except for urPlatformCreateWithNativeHandle, which needs a spec change: #1068

[jandres742]

@bmyates : why is that we need this kind of logic in the L0 loader, of translating driver handles to L0 handles? Do you see any risks on removing this code in the UR loader?

[pbalcer]

The loader handles are there to find the appropriate adapter-level function to call. The alternative is to either make all functions accept an adapter handle as one of the arguments or create a way to map normal handles to adapter handles.

This mechanism is not used when there's only one adapter, see #355.

[alexbatashev]

The alternative is to either make all functions accept an adapter handle as one of the arguments or create a way to map normal handles to adapter handles.

There's no need to. Most interop calls accept a context, which has the right adapter, and this patch exactly changes the behavior to find the adapter in the context, rather the native handle. The only notable exception is platform: #1068. That one certainly needs an adapter as an argument.

[pbalcer]

There's no need to.

I know, I agree. I was replying to Jamie's question.

[jandres742]

thanks @pbalcer , @alexbatashev . Ah, this is only for native handles. That's ok then.

+1

[pbalcer]

Please also modify get_adapter_handles so that it doesn't return native handles. This will remove the now unused object factories.

[alexbatashev]

Please also modify get_adapter_handles so that it doesn't return native handles. This will remove the now unused object factories.

I did try to change it to:

def get_adapter_handles(specs):
    objs = []
    for s in specs:
        for obj in s['objects']:
            if obj_traits.is_handle(obj) and not (obj_traits.is_loader_only(obj) or 'native' in obj['name']):
                objs.append(obj)

    return objs

But the compilation would fail with the following error:

../source/loader/ur_ldrddi.cpp:368:26: error: ‘ur_native_object_t’ does not name a type; did you mean ‘ur_device_object_t’?
  368 |         reinterpret_cast<ur_native_object_t *>(hNativePlatform)->dditable;
      |                          ^~~~~~~~~~~~~~~~~~
      |                          ur_device_object_t
../source/loader/ur_ldrddi.cpp:368:45: error: expected ‘>’ before ‘*’ token
  368 |         reinterpret_cast<ur_native_object_t *>(hNativePlatform)->dditable;
      |                                             ^
../source/loader/ur_ldrddi.cpp:368:45: error: expected ‘(’ before ‘*’ token
  368 |         reinterpret_cast<ur_native_object_t *>(hNativePlatform)->dditable;

Am I doing something wrong?

[pbalcer]

Am I doing something wrong?

No, I think it's all correct. The failure is because platform native handles are special-cased for now. This particular change will have to wait until the spec is fixed.

[alexbatashev]

Am I doing something wrong?

No, I think it's all correct. The failure is because platform native handles are special-cased for now. This particular change will have to wait until the spec is fixed.

Well, I guess, that is the reason why we can't remove ur_native_handle_t factory for now. I don't see a way to leave it only for urProgramCreateWithNative.

[codecov-commenter]

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (329dda6) 16.14% compared to head (8b1bfc9) 16.17%.

Files	Patch %	Lines
source/loader/ur_ldrddi.cpp	0.00%	9 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1067      +/-   ##
==========================================
+ Coverage   16.14%   16.17%   +0.03%     
==========================================
  Files         220      220              
  Lines       30792    30740      -52     
  Branches     3481     3481              
==========================================
+ Hits         4972     4973       +1     
+ Misses      25769    25716      -53     
  Partials       51       51              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.