Skip to content

[ meson ] remove hard code path in meson script @open sesame 12/23 15:55 #2833

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

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

EunjuYang
Copy link
Contributor

  • This PR reflects the previous comment by @myungjoo [link]
    • removing hard code external path in meson script
    • Since BiQGEMM is header-only lib, I skipped the step to search from pkgconfig
  • In the previous version, meson.build contains hard coded path for the BiQGEMM enablement, which is undesirable.
  • This commit updates the meson build script including the top meson and tensor meson by adding meson_option and the step to search the default path.

Self-evaluation:

Build test: [X]Passed [ ]Failed [ ]Skipped
Run test: [X]Passed [ ]Failed [ ]Skipped

Copy link
Member

@skykongkong8 skykongkong8 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

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

LGTM!

meson.build Outdated
# This meson tries to find its installation.
# 1. Checking /usr/include/
# 2. Checking path meson-option specifies
biqgemm_path = '/usr/include/BiQGEMM'
Copy link
Member

Choose a reason for hiding this comment

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

Refer to https://mesonbuild.com/Builtin-options.html

biggemm_path  = join_paths(prefix, includedir, `BiQGEMM`)

If someone cross-compiles nntrainer with biqgemm, this is going to be critical.

@@ -80,7 +80,7 @@ if get_option('enable-biqgemm')
tensor_headers += 'bcq_tensor.h'
tensor_sources += 'bcq_tensor.cpp'
nntrainer_inc += biqgemm_inc
nntrainer_inc_abs += meson.source_root() / '..' / 'BiQGEMM'
nntrainer_inc_abs += meson.source_root() / biqgemm_path
Copy link
Member

Choose a reason for hiding this comment

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

fs = import('fs')
if fs.is_absolete(biqgemm_path)
  nntrainer_inc_abs += biqgemm_path
else
  nntrainer_inc_abs += meson.source_root() / biqgemm_path
endif

Copy link
Contributor Author

@EunjuYang EunjuYang Dec 23, 2024

Choose a reason for hiding this comment

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

Thank you for the review. I missed the case. I updated the commit based on your comments. Thank you :)

@EunjuYang EunjuYang force-pushed the build/biqgemm_dependency branch from f6ac831 to 3f4b6b4 Compare December 23, 2024 01:55
Copy link
Member

@SeoHyungjun SeoHyungjun left a comment

Choose a reason for hiding this comment

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

LGTM

- In the previous version, meson.build contains absolute path for the
  BiQGEMM enablement, which is undesirable.
- This commit updates the meson build script including the top meson and
  tensor meson by adding meson_option and the step to search the default
path.

Self-evaluation:

Build test: [X]Passed [ ]Failed [ ]Skipped
Run test: [X]Passed [ ]Failed [ ]Skipped

Signed-off-by: Eunju Yang <ej.yang@samsung.com>
@EunjuYang EunjuYang changed the title [ meson ] remove hard code path in meson script [ meson ] remove hard code path in meson script @open sesame 12/23 15:55 Dec 23, 2024
@jijoongmoon jijoongmoon merged commit 2e3820d into nnstreamer:main Jan 2, 2025
20 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants