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

Fixes to allow building ETISS Plugins out-of tree #150

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

PhilippvK
Copy link
Member

@PhilippvK PhilippvK commented Jul 4, 2024

Docs for how to build plugins out-of-tree will follow.
(QVanillaAccelerator depends on https://github.com/tum-ei-eda/etiss/commits/patch_sysplugs_vanilla/ which was never merged to main).

@PhilippvK PhilippvK force-pushed the cmake-fixes branch 2 times, most recently from e47f28d to 98a7ea2 Compare July 4, 2024 12:14
@PhilippvK
Copy link
Member Author

Rebased on master

@PhilippvK
Copy link
Member Author

After the latest commit, building etiss plugins can be done without making any changes to the cmakefiles of the plugin:

Just cmake -DETISS_DIR=/path/to/etiss/installation/lib/CMake/ETISS .. and you should be good to go. Now i will need not look into how to load the plugin dynamicallyt at runtime (using list.txt)

@PhilippvK PhilippvK marked this pull request as ready for review July 4, 2024 12:34
@PhilippvK
Copy link
Member Author

PhilippvK commented Jul 4, 2024

CC @mkiessling-ifx @mkiessling89

@PhilippvK
Copy link
Member Author

@wysiwyng lets try to get #149 merged to trigger the ci here...

@PhilippvK
Copy link
Member Author

CI passed, any comments @wysiwyng ?

@wysiwyng
Copy link
Contributor

Looks good from a code standpoint, please add documentation. Currently it is not obvious how the plugins can be used in ETISS after they were built.

Overall, additional work needs to be put in to make ETISS actually relocatable.

Before, etiss would just print a warning due to passing a nullptr to addPlugin:

```
=== Setting up plug-ins ===
ETISS: Info:   Adding Plugin QVanillaAcceleratorT2

ETISS: Warning: etiss::CPUCore::addPlugin() called without passing a valid plugin pointer.
        {
                {core0}
        }
```
@PhilippvK
Copy link
Member Author

@wysiwyng I am working on the docs now. Should it be part of the (quite outdated) Docygen documentation or can I just write a markdown file for explaining how to integrate out-of-tree plugins?

@PhilippvK
Copy link
Member Author

PhilippvK commented Aug 28, 2024

@wysiwyng I have just pushed three more commits that are related to Plugins and would like to hear your feedback:

  • acc1747: Do you think raising a fatal error if a plugin could not be found makes sense? (Earlier we were just printing a warning if nullptr is passed to addPlugin which can easily be overlooked)
  • 22eb958: Without this, the automatic lookup of libs relative to etiss_wd does not work if the path specified in the INI does not end with a slash aka. etiss_wd=/tmp/etiss/ would work, while etiss_wd=/tmp/etiss would not.
  • fabeb76: Setting the default value of etiss_wd to . would allow ETISS to lookup for the PluginImpl directory in the path where ETISS is executed (no need to update the INI). Do you think this is desirable or could be prone to errors?

@PhilippvK
Copy link
Member Author

I realized that there are quite some inconsistencies regarding default values of INI settings. Can I look into fixing these in a followup PR? Also there are a lot of places where file paths are build manually using string concatenation. IMHO it would be good to use boost FS everwhere instead, what do you think @wysiwyng?

@PhilippvK
Copy link
Member Author

@wysiwyng Here is the documentation I came up with: https://github.com/tum-ei-eda/etiss-accelerator-plugins/blob/main/README.md

It's currently specific for the etiss-accelerator-plugins repository but the instructions for other types if plugins would be very similar.

@wysiwyng
Copy link
Contributor

@wysiwyng I am working on the docs now. Should it be part of the (quite outdated) Docygen documentation or can I just write a markdown file for explaining how to integrate out-of-tree plugins?

The latter should be good enough for now.

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.

2 participants