oneapi-src / oneAPI-samples

Samples for Intel® oneAPI Toolkits
https://oneapi-src.github.io/oneAPI-samples/
MIT License
915 stars 678 forks source link

FPGA: Add print statement to mem_channel tutorial to avoid cmake warning #2322

Closed justin-rosner closed 3 months ago

justin-rosner commented 3 months ago

Description

This change adds a print statement of the DEVICE_FLAG so that a CMake "unused variable" warning is not emitted.

External Dependencies

Type of change

Please delete options that are not relevant. Add a 'X' to the one that is applicable.

How Has This Been Tested?

Manually compiled the design to reports to make sure that the cmake warning is no longer displayed to users and that the proper flags are still being passed to the design. When users run cmake they will now see output that looks something like the following:

-- The CXX compiler identification is IntelLLVM 2024.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /p/psg/swip/hld/r/sycl/rel/20240417/linux64/compiler/latest/bin/icpx - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring the design with the following target: ofs_n6001
    DEVICE_FLAG set to Agilex7
-- Additional USER_FPGA_FLAGS=
-- Additional USER_FLAGS=
-- Additional USER_INCLUDE_PATHS=../../../include
-- Additional USER_LIB_PATHS=
-- Additional USER_LIBS=
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/jrosner/stella/external/oneapi-src/oneAPI-samples/DirectProgramming/C++SYCL_FPGA/Tutorials/Features/mem_channel/build
justin-rosner commented 3 months ago

I don't think you can just unilaterally remove this check, since the DEVICE_FLAG variable is checked on line 71 of the CMakeLists.txt.

It looks like this is a documented CMake issue: https://gitlab.kitware.com/cmake/cmake/-/issues/18152. As such, I'm not sure if there is anything that we can do to our code sample to account for this while still keeping the functional behaviour the same. Do you think it is acceptable to have this warning be emitted when users are compiling for unknown boards, e.g. the new Arrow Creek boards?

whitepau commented 3 months ago

you can just put the check before line 71 so it only happens just before the CMake script tries to use DEVICE_FLAG.

If you want the check to happen universally, then i suppose the warning is not a big deal.

justin-rosner commented 3 months ago

you can just put the check before line 71 so it only happens just before the CMake script tries to use DEVICE_FLAG.

If you want the check to happen universally, then i suppose the warning is not a big deal.

I still think that we want the check to happen universally, so I'll just update the regtest and close out this PR. Thanks Paul.

justin-rosner commented 3 months ago

I've reopened this as it turns out that this was not a bug in CMake but rather due to the fact that -DDEVICE_FLAG was never being used if it was passed in conjunction with -DDPART=INTERLEAVING. This new if-else logic covers all of the old cases, while avoiding the unused variable warning.

whitepau commented 3 months ago

Does printing the value of DEVICE_FLAG also silence the warning?