llvm icon indicating copy to clipboard operation
llvm copied to clipboard

Fix error handling in `device_impl::getCurrentDeviceTime()`

Open againull opened this issue 7 months ago • 1 comments

Describe the bug

Currently there is some workaround:

    auto Result =
        getAdapter()->call_nocheck<UrApiKind::urDeviceGetGlobalTimestamps>(
            Device, DeviceTime, HostTime);
    if (Result == UR_RESULT_ERROR_INVALID_OPERATION) {
      // NOTE(UR port): Removed the call to GetLastError because  we shouldn't
      // be calling it after ERROR_INVALID_OPERATION: there is no
      // adapter-specific error.
      throw detail::set_ur_error(
          sycl::exception(
              make_error_code(errc::feature_not_supported),
              "Device and/or backend does not support querying timestamp."),
          UR_RESULT_ERROR_INVALID_OPERATION);
    } else {
      getAdapter()->checkUrResult<errc::feature_not_supported>(Result);
    }
  };

Backend which doesn't support the API is supposed to return UR_RESULT_ERROR_UNSUPPORTED_FEATURE, see https://github.com/intel/llvm/pull/18735#discussion_r2135212515

againull avatar Jun 09 '25 15:06 againull

Is this a UR bug or SYCL RT bug? The way I read it seems that UR adapters should be returning something different, but I'm not sure.

aelovikov-intel avatar Jun 10 '25 14:06 aelovikov-intel

Is this a UR bug or SYCL RT bug? The way I read it seems that UR adapters should be returning something different, but I'm not sure.

I believe it is a UR bug since UR_RESULT_ERROR_INVALID_OPERATION is not in a list of error codes documented for urDeviceGetGlobalTimestamps https://oneapi-src.github.io/unified-runtime/core/api.html#urdevicegetglobaltimestamps. In UR code I see that it just converts backend error code to UR error code. So they have to track & convert this error or officially claim as expected outcome. @againull for what backend have you observed UR_RESULT_ERROR_INVALID_OPERATION error?

KseniyaTikhomirova avatar Jul 22 '25 09:07 KseniyaTikhomirova