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

Disable -mfpu=neon for aarch64 #4098

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Disable -mfpu=neon for aarch64 #4098

merged 1 commit into from
Jul 11, 2023

Conversation

hesmar
Copy link
Contributor

@hesmar hesmar commented Jul 11, 2023

This PR removes the -mfpu command line argument on aarch64, because it is not available in gcc on this architecture.
See: https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html

Copy link
Member

@stweil stweil left a comment

Choose a reason for hiding this comment

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

I'd prefer to add this code at move the whole if block to line 262 because I think that looks simpler and clearer:

  check_cxx_compiler_flag("-mfpu=neon" HAVE_NEON)
  if(HAVE_NEON)
    set(NEON_COMPILE_FLAGS "-mfpu=neon")
    [...]
  endif

Disable -mfpu command line argument on aarch64, because it is not available in gcc (aarch64)
@hesmar
Copy link
Contributor Author

hesmar commented Jul 11, 2023

At the moment, I only moved the NEON_COMPILE_FLAGS up because I am not sure if it is safe to set the ENABLE_LTO only for arm. If you say that this is safe, I can update the PR again.

@stweil
Copy link
Member

stweil commented Jul 11, 2023

The comment says that link time optimization had a problem on arm/RBPi, therefore I think it was only for 32 bit ARM. Disabling LTO for all aarch64 builds does not look reasonable for me. But that can be handled in a separate pull request.

Copy link
Member

@stweil stweil left a comment

Choose a reason for hiding this comment

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

Looks good, thank you.

@stweil stweil merged commit f49b6e9 into tesseract-ocr:main Jul 11, 2023
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