Skip to content

Exclude -lrt on Android platforms with Bionic libraries#755

Merged
fgvanzee merged 3 commits intoflame:masterfrom
leekillough:android_bionic
Jul 27, 2023
Merged

Exclude -lrt on Android platforms with Bionic libraries#755
fgvanzee merged 3 commits intoflame:masterfrom
leekillough:android_bionic

Conversation

@leekillough
Copy link
Copy Markdown
Collaborator

Fixes #752, by excluding -lrt when Android Bionic system libraries are detected with the __BIONIC__ preprocessor macro.

This is not a BLIS configure parameter per se; it is just a modification to the Makefile build system to exclude adding -lrt during Linux builds if an Android system is detected.

This needs to be tested thoroughly. I do not have an Android system to test it with. It is fairly conservative, in that it should not change the behavior of BLIS builds unless __BIONIC__ is defined, and as long as $(CC) is executable during the parsing of common.mk.

Cc: @leo4678

@fgvanzee
Copy link
Copy Markdown
Member

Thanks for this PR, @leekillough. It looks good to me.

@leo4678 Once you confirm that this addresses your situation, I'll merge.

@leekillough
Copy link
Copy Markdown
Collaborator Author

I wonder where the bionic.h header file should be located; I don't want to clutter the top-level BLIS directory.

Alternatively, I even thought about making the GNU Makefile automatically create the bionic.h file on-the-fly. I think there are some bash "heredoc" features which make this easier, but it's hard to put them inside of a Makefile, and it might assume that bash or another shell is the underlying shell which make uses.

So maybe a bash script which is invoked by the Makefile would be simpler. Again, I wouldn't want it to clutter the top-level BLIS directory.

@fgvanzee
Copy link
Copy Markdown
Member

I appreciate your thoughtfulness on this, @leekillough. I also would prefer to not add the file to the top-level directory if at all possible.

@devinamatthews
Copy link
Copy Markdown
Member

Why not put this in configure?

@leekillough
Copy link
Copy Markdown
Collaborator Author

Why not put this in configure?

It is not significant enough of a change to warrant a new configure option, and if it is added to configure, it adds more testing requirements.

It's more like: "For Linux builds, regardless of target architecture, we normally link with -lrt, but on Android, this library's features are implicit in Bionic, and -lrt cannot be explicitly linked with. So on Android Linux builds, we make an exception for -lrt, but keep everything else the same, including configure".

I'd like a suggestion of where best to place the bionic.h file so that it doesn't clutter the main directory.

@fgvanzee
Copy link
Copy Markdown
Member

I'd like a suggestion of where best to place the bionic.h file so that it doesn't clutter the main directory.

How about creating a new directory within build/detect and placing it there?

leekillough and others added 2 commits July 20, 2023 19:45
Details:
- Added entry for leo4678 to acknowledge their opening of issue flame#752.
@fgvanzee fgvanzee merged commit 2db31e0 into flame:master Jul 27, 2023
fgvanzee added a commit that referenced this pull request Aug 15, 2023
Details:
- Commit 2db31e0 (#755) inserted logic into common.mk that attempts to
  preprocess build/detect/android/bionic.h to determine whether the
  __BIONIC__ macro is defined (in which case -lrt should not be included
  in LDFLAGS). However, the path to bionic.h was encoded without regard
  to DIST_PATH, and so utilizing common.mk anywhere that isn't the top-
  level directory (such as in the testsuite directory) resulted in a
  compiler error:

    gcc: error: build/detect/android/bionic.h: No such file or directory
    gcc: fatal error: no input files
    compilation terminated.

  This commit adds a $(DIST_PATH) prefix to the path to bionic.h so that
  it can be located from other applications' Makefiles that use BLIS
  without error.
fgvanzee added a commit that referenced this pull request Aug 19, 2023
Details:
- Commit 2db31e0 (#755) inserted logic into common.mk that attempts to
  preprocess build/detect/android/bionic.h to determine whether the
  __BIONIC__ macro is defined (in which case -lrt should not be included
  in LDFLAGS). However, the path to bionic.h was encoded without regard
  to DIST_PATH, and so utilizing common.mk anywhere that isn't the top-
  level directory (such as in the testsuite directory) resulted in a
  compiler error:

    gcc: error: build/detect/android/bionic.h: No such file or directory
    gcc: fatal error: no input files
    compilation terminated.

  This commit adds a $(DIST_PATH) prefix to the path to bionic.h so that
  it can be located from other applications' Makefiles that use BLIS's
  makefile fragments.
fgvanzee added a commit that referenced this pull request May 22, 2024
Details:
- Commit 2db31e0 (#755) inserted logic into common.mk that attempts to
  preprocess build/detect/android/bionic.h to determine whether the
  __BIONIC__ macro is defined (in which case -lrt should not be included
  in LDFLAGS). However, the path to bionic.h was encoded without regard
  to DIST_PATH, and so utilizing common.mk anywhere that isn't the top-
  level directory (such as in the testsuite directory) resulted in a
  compiler error:

    gcc: error: build/detect/android/bionic.h: No such file or directory
    gcc: fatal error: no input files
    compilation terminated.

  This commit adds a $(DIST_PATH) prefix to the path to bionic.h so that
  it can be located from other applications' Makefiles that use BLIS's
  makefile fragments.
- (cherry picked from commit fa6a9b2)

Set thrcomm timpl_t id inside init functions. (#766)

Details:
- Previously, the timpl_t id being used when a thrcomm_t is being
  initialized was set within the bli_thrcomm_init() dispatch function
  after the timpl_t-specific bli_thrcomm_init_*() function returned. But
  it just occurred to me that each bli_thrcomm_init_*() function already
  intrinsically knows its own timpl_t value. This commit shifts the
  setting of the thrcomm_t.ti field into the corresponding
  bli_thrcomm_init_*() function for each timpl_t type (e.g. single,
  openmp, pthreads, hpx).
- Removed long-deprecated code dating back nearly 10 years.
- Whitespace changes
- Comment updates.
- (cherry picked from 634e532)

Small fixes/improvements to docs/Multithreading.md. (#764)

Details:
- Added reminders that #include "blis.h" must be added to source files
  in order to access BLIS API function prototypes. Thanks to Barry Smith
  for suggesting this improvement.
- Fixed pre-existing typos.
- CREDITS file update.
- (cherry picked from 3cf17b4)

CREDITS file update.

Details:
- Thanks to Igor Zhuravlov for PR #753 (commit 915daaa).
- (cherry picked from dbc7981)

Fix typos in docs + example code comments. (#753)

Details:
- Fixed various typos in API documentation in docs/BLIS*API.md and
  comments in the source code examples within examples/?api/*.c.
- (cherry picked from 915daaa)

Exclude -lrt on Android with Bionic libraries. (#755)

Details:
- Added build/detect/android/bionic.h header to test whether the
  __BIONIC__ cpp macro is defined.
- In common.mk, only add -lrt to LDFLAGS when Bionic is not present.
- CREDITS file update.
- (cherry picked from 2db31e0)

Small fixes to support hpx in the testsuite (#759)

Details:
- Minor changes to test_libblis.c to support hpx.
- (cherry picked from 22ad8c1)

Auto-detect the RISC-V ABI of the compiler and use -mabi= during RISC-V Build
s (#750)

Details:
- Generate a build error if there is a 32/64-bit mismatch between the
  RISC-V ABI or architecture and the BLIS configuration selected.
- Handle Q, Zicsr, ZiFencei, Zba, Zbb, Zbc, Zbs and Zfh extensions in
  the RISC-V architecture auto-detection. ZiFencei and Zicsr is not
  detectable with built-in RISC-V macros right now.
- ZiFencei is not important for BLIS because doesn't it have
  Just-In-Time compilation or self-modifying code, and Zicsr is implied
  by the floating-point extensions, which are required for good
  performance in BLIS.
- Move RISC-V autodetect header files to build/detect/riscv/.
- (cherry picked from c91b41d)

Rewrote regen-symbols.sh (gen-libblis-symbols.sh). (#751)

Details:
- Wrote an alternative to regen-symbols.sh, gen-libblis-symbols.sh,
  that generates a list of exported symbols from the monolithic blis.h
  file rather than peeking inside of the shared object via nm. (This new
  script lives in the 'build' directory and the older script has been
  retired to build/old.) Special thanks to Devin Matthews for authoring
  gen-libblis-symbols.sh.
- Added a 'symbols' target to the top-level Makefile which will refresh
  build/libblis-symbols.def, with supporting changes to common.mk.
- Updates to build/libblis-symbols.def using the new symbol-generating
  script.
- (cherry picked from a0b04e3)

Fix 1m enablement for herk/her2k/syrk/syr2k. (#743)

Details:
- Ever since 28b0982, herk, her2k, syrk, and syr2k have been implemented
  in terms of the gemmt expert API. And since the decision of which
  induced method to use (1m or native) is made *below* the level of the
  expert API, executing any of {herk,her2k,syrk,syr2k} results in BLIS
  checking the enablement status for gemmt.
- This commit applies a band-aid of sorts to this issue by modifying
  bli_l3_ind_oper_get_enable() and bli_l3_ind_oper_set_enable() so that
  any attempts to query or modify the internal enablement status for
  herk, her2k, syrk, or syr2k instead does so for gemmt.
- This solution isn't perfect since, in theory, the user could enable 1m
  for, say, herk but then disable it for syrk, and then be confused when
  herk runs via native execution. But we don't anticipate that users
  modify 1m enablement at the operation level, and so in practice this
  solution is likely fine for now.
- (cherry picked from 89b7863)

add nvhpc compiler support (#719)

Add detection of the NVIDIA nvhpc compiler (`nvc`) in `configure`,
and adjust some warning options in `config.mk`. Currently, no
specific options for `nvc` have been added in the relevant
configurations so it may not be usable without further tweaks.
- (cherry picked from 138de3b)
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.

How to cross-compile libraries for use on Android?

3 participants