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

Addressing the hard float ABI support removal from Android NDK #1215

Closed
amiasato-zz opened this issue Jun 28, 2017 · 10 comments
Closed

Addressing the hard float ABI support removal from Android NDK #1215

amiasato-zz opened this issue Jun 28, 2017 · 10 comments
Milestone

Comments

@amiasato-zz
Copy link

Since the r12 release of the Android NDK, hard float ABIs are no longer supported. This problem was aggravated starting from the r15 version due to the removal of the libm_hard from the development kit.

From what I've gathered, the develop branch already addresses this issue by supporting the ARM_SOFTFP_ABI flag, recently incorporating changes for the arm_soft_fp_abi branch. However, there are still a number of issues such as #1168 and #1145 which show that defaulting to -mfloat-abi=hard flag can provoke a lot of confusion. Also, in #1168 it is highlighted that even though the ARM_SOFTFP_ABI flag is supported, real support is limited to single float operations.

Given this:

  • Does it make sense to change ARMV7 builds, making them use naive C loops just as ARMV5 does? Why does OpenBLAS not support sofftp with ARMv7 #777
  • Or should the ARMV7 only use the naive implementations for double precision, while still benefiting from single precision assembly code?
  • Would there be any difference in performance when explicitly stating to the compiler that we are targeting an ARMV7 arch instead of an ARMV5 for naive implementations?
  • Should the project default the ARM_SOFTFP_ABI to 1?

I'd be very grateful for any comments of people more involved with the project, since I am fairly new to all this cross-compilation stuff.

@brada4
Copy link
Contributor

brada4 commented Jun 29, 2017

Why do you want to remove ARMv7 branch? It is usable on Linux.

@amiasato-zz
Copy link
Author

amiasato-zz commented Jun 29, 2017

I don't want to remove any branches, was it implicit in my reasoning?

What I want is to address the removal of hard float ABI support in Android, which is arguably the most popular ARM platform nowadays. Currently, for OpenBLAS usage in ARMv7-a Android builds, I need to change to either develop or arm_soft_fp_abi branches and compile with TARGET=ARMV5 and ARM_SOFTFP_ABI=1 flags. Otherwise, I get the errors reported in #1168.

@martin-frbg
Copy link
Collaborator

"develop" as the name implies is a work in progress. What is really needed is someone to complete the task started by xianyi on the arm_soft_fp_abi branch - who knows, maybe he will find time to do it himself soon enough. If your code uses a mix of single and double precision calculations, using the non-optimized ARMV5 target would make it run slower than necessary (as you would miss out on what is already implemented through the soft_fp_abi branch). Also the ARMV5 target would imply different compiler options (see Makefile.arm) including ignoring the advanced SIMD instructions ("neon") available in ARMV7. I suspect the best course of action for you if you need a (temporary) solution now might be (your option no.2) to modify the KERNEL.ARMV7 file in kernel/arm so that it uses the generic C implementations found in KERNEL.ARMV5 for all the Dxxx,Cxxx or Zxxx variants of the Sxxx functions that were already modified for softfp. (As noted in #1168 it seems to be trivial to fix ddot by copying a line from what was changed in sdot, but I am unsure what needs to be done to make daxpy work which appears to be what your code tripped over after the fix to ddot. While saxpy and daxpy are implemented through the same .S file, xianyi's fix there is "#ifndef DOUBLE" and I haven't the froggiest idea how it would translate to the DOUBLE case)

@amiasato-zz
Copy link
Author

"I suspect the best course of action for you if you need a (temporary) solution now might be (your option no.2) to modify the KERNEL.ARMV7 file in kernel/arm so that it uses the generic C implementations found in KERNEL.ARMV5 for all the Dxxx,Cxxx or Zxxx variants of the Sxxx functions that were already modified for softfp."

Thanks, will do. Would you like to have it as a temporary fix and a PR? How is testing in this case?

@martin-frbg
Copy link
Collaborator

I suppose it would be possible to use Makefile condition syntax in the KERNEL file so we could have both implementations in the same KERNEL.ARMV7 file with an "ifdef ARM_FLOAT_FP_ABI" to choose between them, so non-Android systems would not be impacted by the changes. Not sure about testing on the target hardware (except seeing that your program reports plausible results instead of crashing) as I think the usual implied "make test" run is suppressed when cross-compiling and the included test programs may not even be built by default in this case. ( I do not have any arm/android hardware with me at the moment to check this, but #922 could be related)

@amiasato-zz
Copy link
Author

I am getting around 30% speedup with the mixed ARMv7 implementation over the pure naive-C compiled with ARMv5 as target, so I guess it's... nice? I'd fetch the pure hard float benchmark if I were able to get timing correct, but int-to-double casts aren't working in this case, and statistics won't print correctly.

@amiasato-zz
Copy link
Author

PR #1226 addresses another issue in newer NDKs which is the GCC support removal. So, if anyone is willing to compile OpenBLAS for usage in their Android projects targeting ARMv7-a archs, I recommend the following procedures:

  • Make sure all your dependencies, including OpenBLAS, are compiled with NDK's clang
  • Make sure all your dependencies are compiled with -mfloat-abi=softfp flag
  • Compile OpenBLAS with the aforementioned fix plus arm: add support for softfp in arm vfp assembly files #1221 (which should be merged in the next release) and with ARM_SOFTFP_ABI=1

@martin-frbg
Copy link
Collaborator

@akira-miasato I tried to merge your comments into the wiki at
https://github.com/xianyi/OpenBLAS/wiki/How-to-build-OpenBLAS-for-Android
could you please check if what I made of it still makes sense ?

@amiasato-zz
Copy link
Author

I used the following compilation commands (more details on PR #1226, with slightly different commands/mnemonics):

export PATH=/path/to/standalone-toolchain/bin:$PATH
make TARGET=ARMV7 HOSTCC=clang-4.0 CC=clang NO_SHARED=1 NOFORTRAN=1 USE_THREAD=0 ARM_SOFTFP_ABI=1 libs

I didn't test with HOSTCC=gcc, as I didn't expect it to work. Older versions of clang, however, should work just fine. If you wish, I can test it all next week.

In short, there should be instructions in the wiki for changing the HOSTCC to the host's clang, but everything else seems fine. If you wish to put more details, here is my listing:

  • Android NDK's lmhard removal is done specifically in the r15 version, which forces the use of the ARM_SOFTFP_ABI=1 flag.
  • GCC is still working in some cases, though support is dropped and compatibility is not guaranteed, specially in future NDK versions.
  • You may compile OpenBLAS wih gcc and link it with clang afterwards, so long as you set USE_THREADS=0, but I advise against it. If USE_THREADS is enabled, you will have issues with stdout/stderr definitions.

@brada4
Copy link
Contributor

brada4 commented Jul 26, 2017

HOSTCC builds some build system bits, but does not influence cross-build target at all. You can use any aged gcc/clang that your system has included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants