Skip to content

The proposed fix for the part number extraction logic from platforminfo #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

The proposed fix for the part number extraction logic from platforminfo #73

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmake/FindVitis.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ function(add_vitis_program
set(PROGRAM_DEPENDS ${PROGRAM_DEPENDS} ${_PROGRAM_DEPENDS})

# Recover the part name used by the given platform
if(NOT PROGRAM_PLATFORM_PART OR NOT "${${PROGRAM_TARGET}_PLATFORM}" STREQUAL "${PROGRAM_PLATFORM}")
if(((NOT DEFINED PROGRAM_PLATFORM_PART) OR (NOT PROGRAM_PLATFORM_PART)) AND (NOT "${${PROGRAM_TARGET}_PLATFORM}" STREQUAL "${PROGRAM_PLATFORM}"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the DEFINED necessary? I thought both undefined and empty would fail the plain check.

IIUC this is effectively turning the OR into an AND?

Copy link
Author

@salehjg salehjg Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I agree.
The only difference of NOT DEFINED VAR and NOT VAR is when VAR is defined, but as an empty string. ORing them will be identical to NOT VAR.

message(STATUS "Querying Vitis platform for ${PROGRAM_TARGET}.")
execute_process(COMMAND ${Vitis_PLATFORMINFO} --platform ${PROGRAM_PLATFORM} -jhardwarePlatform.board.part
OUTPUT_VARIABLE PLATFORM_PART)
Expand All @@ -485,6 +485,7 @@ function(add_vitis_program
set(PROGRAM_PLATFORM_PART ${PLATFORM_PART} CACHE INTERNAL "")
endif()
endif()
message(STATUS "Deduced PROGRAM_PLATFORM_PART: ${PROGRAM_PLATFORM_PART}")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should print the internal CMake variable name

if(NOT PROGRAM_PLATFORM_PART)
message(WARNING "Xilinx platform ${PROGRAM_PLATFORM} was not found. Please consult \"${Vitis_PLATFORMINFO} -l\" for a list of installed platforms.")
else()
Expand Down