Skip to content

runtimes: added check_compile to detect libpfm4-dev#50

Merged
guillon merged 1 commit intoxtc-tools:mainfrom
liamsemeria:dev/sliam/readme-update
Mar 3, 2026
Merged

runtimes: added check_compile to detect libpfm4-dev#50
guillon merged 1 commit intoxtc-tools:mainfrom
liamsemeria:dev/sliam/readme-update

Conversation

@liamsemeria
Copy link
Contributor

@liamsemeria liamsemeria commented Feb 16, 2026

Motivation:
xtc wasn't working without it installed because of perfmon includes in the runtime.

@liamsemeria liamsemeria marked this pull request as ready for review February 16, 2026 09:19
@ElectrikSpace
Copy link
Contributor

I think we need to fix #43 first.
New CPUs are not necessarily supported by the distributed versions of libpfm4-dev on stable linux distros.

@qaco
Copy link
Contributor

qaco commented Feb 26, 2026

I think we need to fix #43 first. New CPUs are not necessarily supported by the distributed versions of libpfm4-dev on stable linux distros.

Ok could you open a PR for #43 (with an explicit error message) ?

@guillon
Copy link
Member

guillon commented Feb 26, 2026

@liamsemeria Can you add the motivation, use case for this PR in the description?

Originally, I wanted to have the least set of dependencies for XTC to work, i.e. if not installed/available the hw counter just return 0.

Do you think there would be a motivation to change this?

@liamsemeria
Copy link
Contributor Author

@guillon I updated the motivation. When installing xtc on my computer, I'm pretty sure none of the tests were passing without it installed, and simple xtc examples weren't working.

@guillon
Copy link
Member

guillon commented Feb 27, 2026

@liamsemeria ok, I understand. I think that th problem comes from the fact that one may have:

  • libpfm4 installed
  • bit not libpfm4-dev
    In this case, I guess that XTC detects the library, but as the includes are not installed, it fails.

So basically, what is wrong:

  • we should not detect the presence of libpfm4-dev by a library query only, but also by a test-compile including the header
    We could:
  • add in utils/... a function to test the correct compilation of a .c file (with optional libraries)
  • use it to check for a correctly installed libpfm4-dev with something like: has_pfm4 = check_compile("#include <...>\nint main() { return 0; }\n", libs="pfm4")

Do you think you could contribute this?

@liamsemeria
Copy link
Contributor Author

@guillon Yeah I can. Seems like I would then use that similar to has_pfm in src/xtc/runtimes/host/runtime.py

@guillon
Copy link
Member

guillon commented Feb 27, 2026

@guillon I updated the motivation. When installing xtc on my computer, I'm pretty sure none of the tests were passing without it installed, and simple xtc examples weren't working.

Yes, as explained what I did there was not actually sufficient to detect a "dev" package, you need to test the presence of the lib and of the include files.

@guillon guillon added the enhancement New feature or request label Feb 27, 2026
@liamsemeria liamsemeria force-pushed the dev/sliam/readme-update branch from 428dbd8 to 7b91b0f Compare March 2, 2026 16:53
@liamsemeria
Copy link
Contributor Author

@guillon I added the check_compile, it fixed the issue and all tests pass independent of whether or not a libpfm4 is installed/

@guillon guillon changed the title README: put libpfm4-dev as not optional runtimes: added check_compile to detect libpfm4-dev Mar 2, 2026
@liamsemeria liamsemeria force-pushed the dev/sliam/readme-update branch from 7b91b0f to 632886e Compare March 3, 2026 09:20
@liamsemeria
Copy link
Contributor Author

@guillon fixed the double import

@guillon guillon self-requested a review March 3, 2026 10:49
Copy link
Member

@guillon guillon left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this change! Great

@guillon guillon merged commit 912fe8d into xtc-tools:main Mar 3, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants