Reverse UMF_BUILD_LEVEL_ZERO_PROVIDER and UMF_BUILD_CUDA_PROVIDER #825
Conversation
UMF_BUILD_LEVEL_ZERO_PROVIDER and UMF_BUILD_CUDA_PROVIDER are OFF by default from now on. Signed-off-by: Lukasz Dorau <[email protected]>
|
please update run_build.sh also |
|
Why are we disabling CUDA and L0 by default? They should be part of libumf.so by default and they have no buld/runtime dependencies. |
It is OK now. |
|
@vinser52 they will still be a part of the build for oneAPI. |
But it is a question about defaults whether we want developer to disable CUDA and L0 on the system where download is not possible or do we want to enable it in all other cases. I prefer to have them ON by default and disable only if build system cannot download headers. |
But how to check this? The problem occurs on intel/llvm (see intel/llvm#15686). Please note that these CMAKE options are set by UR in this case, not directly in llvm. |
I see… Is it OK to have UMF build without CUDA and L0 providers for intel/llvm? Today UR uses its own implementation of L0 and CUDA providers, but in a long term UR should use ones from UMF. I think the right solution in that case is to have L0/CUDA headers to be part of UMF repo. |
I'm not sure. I think there are scenarios where they already have these headers on their system. For L0 we got a CMAKE var to handle this: UMF_LEVEL_ZERO_INCLUDE_DIR - it is currently used in UR. We should add a similar var for CUDA |
There was a problem hiding this comment.
We should do what intel/llvm and UR does - make extra features opt-in. UR doesn't even build the level-zero adapter by default.
We can't be breaking other people's builds with extraneous dependencies that they don't want or need.
Once the CUDA adapter is ready, there should be a cmake variable that specifies where are the headers. UR will supply the one it uses when the CUDA adapter is built (which is also an opt-in), and this will avoid the download.
What is the reason to use user provided headers? As I can understand, there are 2 possible cases:
|
So that we avoid a download from a third-party. |
Sorry, I was not clear. My question was about option when we just commit CUDA/L0 headers to our repo to avoid download step. |
Why would we do that? At that point, you are responsible for redistributing the cuda headers, which complicates the release process. Adding a variable to specify the headers is exactly what other components do (pretty much all the adapters in UR have this option). |
|
I think what @pbalcer suggested is doable but from my point of view it is overcomplication (vs just commiting the headers to the repo) and for me it is not clear for what reasons (I mean what is the benefits to use user provided headers). |
I am worried that it might cause the situation when we will have two versions of |
What exactly is the problem though? We would align the behavior of UMFs build system to that of the rest of the stack. Checking-in a third-party component (which is not that small) with a proprietary license, adding the extra burden of maintaining that, for I'm not exactly sure what purpose? Especially since, in the compiler build from where cuda is enabled, those headers already exist. We don't need that extra copy.
The moment you check-in a third-party component, you are redistributing it. |
Ok, I got your point. We can be aligned with UR build system. But my initial question was about defaults. Why should we change defaults and disable L0/CUDA by default? If UR always knows if it build L0/CUDA adapter or not than UR always can pass the ON/OFF options for L0/CUDA providers. |
|
And to be clear, I am not strictly against this changes. I just raised questions/concerns. |
On that I don't have a strong opinion. But we need to either change the defaults here or always make sure we disable all we don't need in UR when we do FetchContent. My slight preference would be to disabling everything UMF (just to be consistent with UR). |
I think you are biased because you are working on UR :) |
|
Please note that we download L0 headers only when tests and examples are built (which is disabled by default). So L0 headers wouldn't be fetched by default while L0 provider would be built. However this is not true for CUDA provider. We use a lot CUDA definitions in the provider code (far more than in L0), so my decision here was to just include cuda.h - maybe if we define them similar to what we have in include/umf/providers/provider_level_zero.h we could disable fetching CUDA headers in the default path |
I did not know that. I thought that we download L0 headers to build the L0 provider as well. In that case it is another point not to disable (at least L0 provider) by default. Regarding CUDA we can investigate how it would be to get rid of |
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Fetch L0 loader only if needed i.e.: if building L0 provider is ON and L0 headers are not provided by the user (via setting UMF_LEVEL_ZERO_INCLUDE_DIR). Fetch CUDA only if needed i.e.: if building CUDA provider is ON and CUDA headers are not provided by the user (via setting UMF_CUDA_INCLUDE_DIR). Ref: oneapi-src#825 Fixes: oneapi-src#827 Signed-off-by: Lukasz Dorau <[email protected]>
Description
UMF_BUILD_LEVEL_ZERO_PROVIDERandUMF_BUILD_CUDA_PROVIDERareOFFby default from now on.Checklist