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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve precompiled libraries handling #512

Merged
merged 1 commit into from Feb 11, 2020

Conversation

facchinm
Copy link
Member

General change:

  • library compilation bails out if the precompiled object is found.
    This allows mixed libraries that fallback using sources if no suitable precompiled object is found

ARM float specification change:

  • Cortex M4+ allows specifying different flags for floating point ABI
    This patch introduces a second level of subfolder that MUST be used if -mfpu or -mfloat-abi are specified on the commandline
    Since there's no clear specification (unlike build.mcu), the values are extracted from c++ recipe

    For example, for a target which commandline contains -mfloat-abi=hard -mfpu=fpv4-sp-d16 , the precompiled search path will be
    $libfolder/cortex-m4/fpv4-sp-d16-hard
    If that folder doesn't exist the library will be compiled from sources

  • Open questions:
    While reviewing this I found a code path just to add static libraries with non standard name (not starting with "lib" ). Should we keep it or not? @cmaglie
    I don't like the name of the subfolder but it's super precise. We should be able to get away with -float-abi=xxx only but I'm not 100% sure. Experts' advise are welcome 馃檪

Fixes arduino/arduino-builder#256

General change:
 - library compilation bails out if the precompiled object is found.
   This allows mixed libraries that fallback using sources if no suitable precompiled object is found

ARM float specification change:
 - Cortex M4+ allows specifying different flags for floating point ABI
   This patch introduces a second level of subfolder that MUST be used is -mfpu or -mfloat-abi are specified on the commandline
   Since there's no clear specification (unlike build.mcu), the values are extracted from c++ recipe

   For example, for a target which commandline contains `-mfloat-abi=hard -mfpu=fpv4-sp-d16` , the precompiled search path will be
   `$libfolder/cortex-m4/fpv4-sp-d16-hard`
   If that folder doesn't exist the library will be compiled from sources

Fixes arduino/arduino-builder#256
@sandeepmistry
Copy link

Unfortunately this is not working for me with the arduino-builder binary @cmaglie provided for me.

For the Arduino Nano 33 BLE, I see the following:

Linking everything together...
/Users/smistry/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-g++ -L/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086 -Wl,--gc-sections -Wall -Wextra -Wl,--as-needed @/Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/ldflags.txt -T/Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/linker_script.ld -Wl,-Map,/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.map --specs=nano.specs --specs=nosys.specs -o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.elf /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_constants.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_main.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_output_handler.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/sine_model_data.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/core/variant.cpp.o -Wl,--whole-archive /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/core/core.a /Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/libs/libmbed.a /Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/libs/libcc_310_core.a /Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/libs/libcc_310_ext.a /Users/smistry/Library/Arduino15/packages/arduino/hardware/mbed/1.1.3/variants/ARDUINO_NANO33BLE/libs/libcc_310_trng.a -Wl,--no-whole-archive -Wl,--start-group -lstdc++ -lsupc++ -lm -lc -lgcc -lnosys -L/Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite/src/cortex-m4 -lm -ltensorflowlite -ltensorflowlite -Wl,--end-group
/Users/smistry/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-objcopy -O binary /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.elf /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.bin
/Users/smistry/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-objcopy -O ihex -R .eeprom /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.elf /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.hex
Multiple libraries were found for "TensorFlowLite.h"
 Used: /Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite
 Not used: /Users/smistry/Documents/Arduino/libraries/micro_speech
 Not used: /Users/smistry/Documents/Arduino/libraries/tensorflow_lite
Using library Arduino_TensorFlowLite at version 1.15.0-ALPHA-precompiled in folder: /Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite 
/Users/smistry/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-size -A /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.elf
Sketch uses 169680 bytes (17%) of program storage space. Maximum is 983040 bytes.
Global variables use 52008 bytes (19%) of dynamic memory, leaving 210136 bytes for local variables. Maximum is 262144 bytes.

when compiling the hello_world example of the TensorFlow Lite Micro library.

however when compiling for the MKR Zero, I get linker errors:

rchiving built core (caching) in: /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_cache_826483/core/core_arduino_samd_arduino_zero_edbg_35d1a284781ccd8b47657e0161ff68e7.a
Linking everything together...
/Users/smistry/Library/Arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-g++ -L/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086 -Os -Wl,--gc-sections -save-temps -T/Users/smistry/Library/Arduino15/packages/arduino/hardware/samd/1.8.4/variants/arduino_zero/linker_scripts/gcc/flash_with_bootloader.ld -Wl,-Map,/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.map --specs=nano.specs --specs=nosys.specs -mcpu=cortex-m0plus -mthumb -Wl,--cref -Wl,--check-sections -Wl,--gc-sections -Wl,--unresolved-symbols=report-all -Wl,--warn-common -Wl,--warn-section-align -o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/hello_world.ino.elf /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_constants.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_main.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_output_handler.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/sine_model_data.cpp.o /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/core/variant.cpp.o -Wl,--start-group -L/Users/smistry/Library/Arduino15/packages/arduino/tools/CMSIS/4.5.0/CMSIS/Lib/GCC/ -larm_cortexM0l_math -lm /var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/core/core.a -Wl,--end-group
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_output_handler.cpp.o: In function `HandleOutput(tflite::ErrorReporter*, float, float)':
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/arduino_output_handler.cpp:46: undefined reference to `tflite::ErrorReporter::Report(char const*, ...)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o: In function `tflite::MicroInterpreter::~MicroInterpreter()':
/Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite/src/tensorflow/lite/experimental/micro/micro_allocator.h:27: undefined reference to `tflite::SimpleMemoryAllocator::~SimpleMemoryAllocator()'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o: In function `setup':
Multiple libraries were found for "TensorFlowLite.h"
 Used: /Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite
 Not used: /Users/smistry/Documents/Arduino/libraries/tensorflow_lite
 Not used: /Users/smistry/Documents/Arduino/libraries/micro_speech
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:59: undefined reference to `tflite::ErrorReporter::Report(char const*, ...)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:68: undefined reference to `tflite::ops::micro::AllOpsResolver::AllOpsResolver()'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:72: undefined reference to `tflite::MicroInterpreter::MicroInterpreter(tflite::Model const*, tflite::OpResolver const&, unsigned char*, unsigned int, tflite::ErrorReporter*)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:76: undefined reference to `tflite::MicroInterpreter::AllocateTensors()'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:78: undefined reference to `tflite::ErrorReporter::Report(char const*, ...)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:83: undefined reference to `tflite::MicroInterpreter::input(unsigned int)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:84: undefined reference to `tflite::MicroInterpreter::output(unsigned int)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o: In function `loop':
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:104: undefined reference to `tflite::MicroInterpreter::Invoke()'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_modified_sketch_508019/hello_world.ino:106: undefined reference to `tflite::ErrorReporter::Report(char const*, ...)'
/var/folders/ks/9_b25vts0nl8nzsgtkt7pkp80000gn/T/arduino_build_153086/sketch/hello_world.ino.cpp.o:(.data._ZZ5setupE20micro_error_reporter+0x0): undefined reference to `vtable for tflite::MicroErrorReporter'
collect2: error: ld returned 1 exit status
Using library Arduino_TensorFlowLite at version 1.15.0-ALPHA-precompiled in folder: /Users/smistry/Documents/Arduino/libraries/Arduino_TensorFlowLite 
exit status 1
Error compiling for board Arduino Zero (Programming Port).

Note the following changes are needed to the sketch:


#include <TensorFlowLite.h>

#undef max // <--- added
#undef min // <--- added

@facchinm
Copy link
Member Author

@sandeepmistry that's because you need a specially crafted version of tflite library with the precompiled directive AND the sources. This one should work 馃檪
Arduino_TensorFlowLite.zip

@sandeepmistry
Copy link

@facchinm of course! Totally my bad :) ... the zip you provided links perfectly for MKR Zero!

@ladyada
Copy link

ladyada commented Dec 16, 2019

hi folks, is there a way i could test the PR?

@ladyada
Copy link

ladyada commented Dec 26, 2019

hihi folks checkin' in again if there's anything i can do to help test this PR

@masci masci added this to the 0.8.0 milestone Dec 31, 2019
@cmaglie
Copy link
Member

cmaglie commented Jan 30, 2020

While reviewing this I found a code path just to add static libraries with non standard name (not starting with "lib" ). Should we keep it or not? @cmaglie

If not needed maybe it's better to remove it, there are known use cases?

Copy link
Member

@cmaglie cmaglie left a comment

Choose a reason for hiding this comment

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

Besides the .a with non-standard name (that was already handled so it's not part of this pr) LGTM

@cmaglie cmaglie merged commit 469b339 into arduino:master Feb 11, 2020
cmaglie added a commit to arduino/arduino-builder that referenced this pull request Feb 11, 2020
- Fix library priority selection (again)
  arduino/arduino-cli#574

- Improve precompiled libraries handling
  arduino/arduino-cli#512
facchinm added a commit to facchinm/arduino-cli that referenced this pull request Mar 10, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
facchinm added a commit to facchinm/arduino-cli that referenced this pull request Mar 10, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
facchinm added a commit to facchinm/arduino-cli that referenced this pull request Mar 24, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
facchinm added a commit to facchinm/arduino-cli that referenced this pull request Apr 20, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
cmaglie pushed a commit to facchinm/arduino-cli that referenced this pull request May 4, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
cmaglie pushed a commit to facchinm/arduino-cli that referenced this pull request May 4, 2020
This patch restores some functionalities broken by arduino#512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* arduino#512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)
cmaglie added a commit that referenced this pull request May 8, 2020
* Fix mixed code precompiled libraries

This patch restores some functionalities broken by #512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* #512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)

* Added test-build for Arduino_TensorFlowLite

* Added another test for Bosch Sensor lib

* Fallback search for precompiled libraries in non-fpu specific directories

* variable renamed

* Moved fixLDFLAG inside compileLibraries

* Inlined FixLDflags

* Using more paths helpers to simplify code

* Added support for "precompiled=full" in library.properties

This flag allow the deployment of a pure precompiled library that ships
also the source code of the precompiled part.
This allows to possibly compile-from-source as a fallback if the library
does not provide a precompiled build for the current architecture.

The "precompiled=true" instead has been ported back to the old behaviour
(for libraries that ships a precompiled binary and support source code
that wraps/uses the precompiled part).

* Updated tests

* Fixed test compile-build on very long paths on Windows

* Removed constants.BUILD_PROPERTIES_COMPILER_LIBRARIES_LDFLAGS

* Add space before "-L" gcc flag to allow multiple precompiled libs to be included

Co-authored-by: Cristian Maglie <c.maglie@arduino.cc>
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.

Precompiled libs feature doesn't account for floating point mcu variants
5 participants