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

meson: avx condition fix. #2902

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

Conversation

myungjoo
Copy link
Member

enable-avx=true is the default option, which breaks the build in other archs in general.

  1. Ignore AVX if arch is not x64/x86.
  2. Reading get_option should be done only once. Fixed for enable-avx.
  3. Corrected the buggy AVX condition.

This fix is required by RISC-V: #2638

Copy link
Contributor

@EunjuYang EunjuYang left a comment

Choose a reason for hiding this comment

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

LGTM !

enable-avx=true is the default option, which breaks the build
in other archs in general.

1. Ignore AVX if arch is not x64/x86.
2. Reading get_option should be done only once. Fixed for enable-avx.
2. Corrected the buggy AVX condition.

This fix is required by RISC-V.

Signed-off-by: MyungJoo Ham <[email protected]>
Copy link
Contributor

@djeong20 djeong20 left a comment

Choose a reason for hiding this comment

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

Overall, LGTM

if avx_enabled == true
tensor_sources += 'blas_avx.cpp'
tensor_headers += 'blas_avx.h'
avx_enabled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
avx_enabled = true

remove?

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.

3 participants