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

Add arm-native build and test on Ubuntu #2897

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

myungjoo
Copy link
Member

We can run arm natively with github-action.
Test fp16 natively on arm machines!

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.

Great to test them automatically on action!

skykongkong8

This comment was marked as abuse.

@skykongkong8
Copy link
Member

skykongkong8 commented Jan 23, 2025

if get_option('enable-fp16') 
  if arch == 'arm'
    error ('FP16/ARM code (blas_neon.cpp) uses armv8.2 instructions. armv7 is not supported.')
  elif arch == 'aarch64' or get_option('platform') == 'android'
    if get_option('enable-neon')
      tensor_sources += 'blas_neon.cpp'
      tensor_headers += 'blas_neon.h'
      subdir('hgemm')
      nntrainer_inc += include_directories('hgemm')
      nntrainer_inc_abs += meson.current_source_dir() / 'hgemm'

      subdir('matrix_transpose_neon')
      nntrainer_inc += include_directories('matrix_transpose_neon')
      nntrainer_inc_abs += meson.current_source_dir() / 'matrix_transpose_neon'
    endif
  endif
endif

matrix_transpose_neon is included like above. Is ubuntu-22.04-arm armv7?
If it is, it should not built with enable-fp16=true

+) I found additional bug. #2832 is merged recently, and I missed to divide fp32 / fp16 matrix transpose with neon. I will fix this accordingly asap -> fixed on #2898

@myungjoo
Copy link
Member Author

It's aarch64.

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.

I have a quick question!
It seems meson libdir at ubuntu_clean_meson_build.yml is being set like below :

--libdir=lib/x86_64-linux-gnu \

would this work with ubuntu-22.04-arm as well?

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.

It looks great! LGTM!

@myungjoo
Copy link
Member Author

I have a quick question! It seems meson libdir at ubuntu_clean_meson_build.yml is being set like below :

--libdir=lib/x86_64-linux-gnu \

would this work with ubuntu-22.04-arm as well?

Nope. that should be fixed. Hardcoding libdir is not a good idea.

We can run arm natively with github-action.
Test fp16 natively on arm machines!

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
libdir, bindir, and includedir are automatically detected by
meson. You usually do not need to configure them if you are not
cross-compiling.

The bindir config is left because it is intentionally using
non-standard path.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
@myungjoo
Copy link
Member Author

@skykongkong8 libdir is fixed. Let's not specify if not needed.

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.

LGTM

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.

4 participants