Skip to content
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.

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

Disable install of aiebu and aie-rt #8655

Merged
merged 6 commits into from
Dec 13, 2024

Conversation

stsoe
Copy link
Collaborator

@stsoe stsoe commented Dec 11, 2024

Problem solved by the commit

Improper install targets pollutes the XRT packages with xaiengine header and static libraries. This PR disables the install target for aie-rt both via aiebu and directly as submodule used by XDP on NPU.

PR updates src/runtime_src/core/common/aiebu to include

PR updates src/runtime_src/aie-rt to be in synch with what aiebu submodule expects after it was updated. It also removes the [binary_dir] used in add_subdirectory for aie-rt. The binary_dir destination is not needed and not commonly used, it also interferes with the override to remove install artifacts, which was added in earlier commits in this PR.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

Strenuous files in /opt/xilinx and in xrt packages

How problem was solved, alternative solutions (if any) and why they were rejected

Add CMake function in src/CMake/utilities.cmake to disable the install target of added subdirectory. The function leverages CMAKE_SKIP_INSTALL_RULES but additional works around a missing feature in CMake.

Risks (if any) associated the changes in the commit

That someone relied on the exported content; very unlikely

What has been tested and how, request additional testing if necessary

Builds comparing install content. Both Linux and Windows and for MCDM.

Improper install targets polutes the XRT packages with xaiengine
header and static libraries.  This PR disables the install target for
both aiebe and aie-rt.

Add CMake function in src/CMake/utilities.cmake to disable
the install target of added subdirectory.  The function works
around a missing feature in CMake.

```
```

Signed-off-by: Soren Soe <[email protected]>
@stsoe stsoe requested a review from rozumx as a code owner December 11, 2024 23:15
@sonals
Copy link
Member

sonals commented Dec 11, 2024

We do need to release aiebu artifacts (the statically linked aiebu-asm and the header files) as part of XRT. We do not need aie-rt as it is not used by end users or model developers.

However, aiebu artifacts are required by end users and model developers.

@stsoe
Copy link
Collaborator Author

stsoe commented Dec 12, 2024

We do need to release aiebu artifacts (the statically linked aiebu-asm and the header files) as part of XRT. We do not need aie-rt as it is not used by end users or model developers.

However, aiebu artifacts are required by end users and model developers.

Alright. Thanks for this note. @sonals
Could you please clarify what aiebu artifacts you are talking about? And which package you want to release what. Below files are contained in a component Runtime package (xrt_202510.2.19.0_22.04-amd64-Runtime.tar.gz), please suggest a better component name or if what files should be contained in which component, I am thinking deployment and development here.

We will need surgery to aiebu in order to prevent it from releasing aie-rt content. Would have been nice had this been addressed when submodules were added; things don't just magically clean up by themselves :-(

opt/xilinx/aiebu/lib/libaiebu.so
opt/xilinx/aiebu/lib/libaiebu_static.a
opt/xilinx/aiebu/lib/aie2/preempt_restore_stx_4x1.bin
opt/xilinx/aiebu/lib/aie2/preempt_restore_stx_4x2.bin
opt/xilinx/aiebu/lib/aie2/preempt_restore_stx_4x4.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_stx_4x1.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_stx_4x2.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_stx_4x4.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_phx_4x1.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_phx_4x2.bin
opt/xilinx/aiebu/lib/aie2/preempt_save_phx_4x4.bin
opt/xilinx/aiebu/lib/aie2/preempt_restore_phx_4x1.bin
opt/xilinx/aiebu/lib/aie2/preempt_restore_phx_4x2.bin
opt/xilinx/aiebu/lib/aie2/preempt_restore_phx_4x4.bin
opt/xilinx/aiebu/lib/aie2/copy_Mem_Tile_to_DDR.bin
opt/xilinx/aiebu/lib/aie2/copy_DDR_to_Mem_Tile.bin
opt/xilinx/aiebu/include/aiebu.h
opt/xilinx/aiebu/include/aiebu_assembler.h
opt/xilinx/aiebu/include/aiebu_error.h
opt/xilinx/aiebu/bin/aiebu-asm

@stsoe stsoe added the do not merge hold off on merging label Dec 12, 2024
@sonals
Copy link
Member

sonals commented Dec 12, 2024

We need to weed out all the bin files for aiebu. But I agree with you that the CMake for aiebu should be updated so aiebu does not blindly release all the artifacts. Let me take a stab at it later this week.

Maybe you can update this PR to disable aie-rt release and leave out the aiebu for now?

@stsoe
Copy link
Collaborator Author

stsoe commented Dec 12, 2024

Maybe you can update this PR to disable aie-rt release and leave out the aiebu for now?

I have to update aiebu to stop the release of aie-rt artifacts that come from aiebu. Are we okay bumping aiebu submodule in XRT so I can include this change in aiebu?

Please check Xilinx/aiebu#34

XRT is expected to release artifacts created by aiebu so reverting change.

The aiebu submodule will be updated once it has gone through cleanup
to remove release of artifacts that are not needed, plus review of
what exactly is needed and why.

Signed-off-by: Soren Soe <[email protected]>
Pull in Xilinx/aiebu#34 to disable release
of aie-rt headers and more.

Signed-off-by: Soren Soe <[email protected]>
Pull in Xilinx/aiebu#35

Signed-off-by: Soren Soe <[email protected]>
@stsoe stsoe removed the do not merge hold off on merging label Dec 12, 2024
@stsoe stsoe requested review from sonals and removed request for rozumx December 12, 2024 19:04
This PR updates the aie-rt submodule at src/runtime_src/aie-rt to be
in synch with what aiebu submodule expects.  The aiebu submodule is
updated in earlier commits of this PR.

This is a mess.  Why two copies of aie-rt?  But nevermind, on windows
aiebu is using src/runtime_src/aie-rt and this version of aie-rt needs
to match what aiebu is expecting.

Signed-off-by: Soren Soe <[email protected]>
The binary_dir destination is not needed and not commonly used, it
also interferes with the override to remove install artifacts, which
was added in earlier commits in this PR.

Signed-off-by: Soren Soe <[email protected]>
@stsoe stsoe merged commit 2ec7849 into Xilinx:master Dec 13, 2024
20 checks passed
@stsoe stsoe deleted the disable_install_aiebu_aiert branch December 13, 2024 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants