oneapi-src / oneDPL

oneAPI DPC++ Library (oneDPL) https://software.intel.com/content/www/us/en/develop/tools/oneapi/components/dpc-library.html
Apache License 2.0
715 stars 112 forks source link

[test][cmake] Support customization of device target options in AOT mode #1661

Closed mmichel11 closed 3 days ago

mmichel11 commented 1 week ago

Our current CMake AOT compilation mode sets -fsycl-targets=spir64_gen for GPU targets and -fsycl-targets=spir64_x86_64 for CPU targets. This is problematic for two reasons: The first is that some backends do not compile to SPIR-V (e.g. CUDA backend), so compiling oneDPL tests for CUDA devices is not possible using our AOT CMake options and requires users to set CXX compiler and linker flag variables themselves. Secondly, the icpx compiler supports specialized target options for ease of use (e.g. intel_gpu_pvc) which cannot be currently passed through our AOT options.

This PR adds a ONEDPL_AOT_TARGETS option defaulted to the current hardcoded variables to resolve these issues. ONEDPL_AOT_TARGETS can be set alongside ONEDPL_AOT_ARCH or just set to a special target option with no need for specifying an architecture. With the defaulted value of ONEDPL_AOT_TARGETS, the previous behavior of just setting ONEDPL_AOT_ARCH is maintained.

I have ran some AOT tests with the following configurations to assess testing with different device types. The Codeplay oneAPI plugin for CUDA and hip backends was used with oneAPI 2024.2.0. GPU AOT compilation:

CPU AOT compilation:

dmitriy-sobolev commented 1 week ago

I see these factors as reasons for removal AOT commands from our CMake:

Note that CMake AOT options affect the tests only, so I assume we are not obliged to raise oneDPL version upon their removal.

Given that their removal will force us using the compiler/backend options directly, we will get that change in the command lines (simplified):

# Helpers, Intel GPU, any GPU architecture
cmake -DONEDPL_USE_AOT_COMPILATION=on <...>

# No helpers, Intel GPU, any GPU architecture (easy to specify a particular one, though)
cmake  -DCMAKE_CXX_FLAGS="-fsycl -fsycl-targets=spir64 -Xs '-march=*'" <...>

# Helpers, other device from other vendor
cmake -DONEDPL_USE_AOT_COMPILATION=on -DONEDPL_AOT_TARGETS=<target> -DONEDPL_AOT_ARCH=<arch> <...>

# No helpers, other device from other vendor
cmake -DCMAKE_CXX_FLAGS="-fsycl -fsycl-targets=<target> -Xs '-march=<arch>'" <...>

The complexity of using of AOT options directly does not look too high to me (especially if you aware of the guide on AOT options) to justify having so much CMake infrastructure and focus in the documentation.

@mmichel11, what do you think about removing AOT CMake options entirely, deprecating them first? If you think it worth consideration, could you ask for the opinion of other developers as suggested above (either online/offline).

mmichel11 commented 4 days ago

I see these factors as reasons for removal AOT commands from our CMake:

  • We should introduce many CMake options to support possible AOT targets.
  • The options are not used much by the developers. It can be confirmed by some kind of polling among the developers.
  • The internal testing system does not use them.

Note that CMake AOT options affect the tests only, so I assume we are not obliged to raise oneDPL version upon their removal.

Given that their removal will force us using the compiler/backend options directly, we will get that change in the command lines (simplified):

# Helpers, Intel GPU, any GPU architecture
cmake -DONEDPL_USE_AOT_COMPILATION=on <...>

# No helpers, Intel GPU, any GPU architecture (easy to specify a particular one, though)
cmake  -DCMAKE_CXX_FLAGS="-fsycl -fsycl-targets=spir64 -Xs '-march=*'" <...>

# Helpers, other device from other vendor
cmake -DONEDPL_USE_AOT_COMPILATION=on -DONEDPL_AOT_TARGETS=<target> -DONEDPL_AOT_ARCH=<arch> <...>

# No helpers, other device from other vendor
cmake -DCMAKE_CXX_FLAGS="-fsycl -fsycl-targets=<target> -Xs '-march=<arch>'" <...>

The complexity of using of AOT options directly does not look too high to me (especially if you aware of the guide on AOT options) to justify having so much CMake infrastructure and focus in the documentation.

@mmichel11, what do you think about removing AOT CMake options entirely, deprecating them first? If you think it worth consideration, could you ask for the opinion of other developers as suggested above (either online/offline).

I think deprecating and removing the AOT options is also a viable option. I expect the AOT options are most important to users who wish to build the tests for non-Intel devices where the AOT flags are required which cannot currently be achieved with our CMake options. We could link to the compiler AOT compilation documentation and the intel/llvm User manual which lists the targets for compilation in our CMake README.

Are there any other opinions on this? @danhoeflinger @julianmi @adamfidel @SergeyKopienko

julianmi commented 4 days ago

Deprecating and removing the AOT flags from our CMake make sense to me. We might want to add AOT compilation for required devices and performance reasons to our documentation (especially https://oneapi-src.github.io/oneDPL/introduction.html#build-your-code-with-onedpl-short).

danhoeflinger commented 3 days ago

I agree that we don't need our own system of how to trigger AOT compilation, and can instead in our documentation mention that this is an option and link to the official documentation for icpx. Users can specify it via the command line options as is described by Dmitriy. Less maintenance for us, and not really more complicated to specify for the user.

As for the question of whether we need to deprecate this as a feature prior to deleting, I'm not sure... probably we don't need to deprecate, but its worth discussing.

mmichel11 commented 3 days ago

Thank you for the feedback, all.

I am in agreement that this solution is probably more trouble than it's worth for our users. I am closing this PR for now and will open a new one with the discussed changes and documentation when ready.