Skip to content

ARM toolchain - add support for small-build #2210

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

Closed
wants to merge 1 commit into from

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Jul 21, 2016

Same as for GCC, if small-build is selected, select microlib library. This
is currently duplicated from microlib. I assume we deprecate microlib toolchain, thus
duplication should be removed (regarding extending cc/asm flags by --microlib, etc).

Tested with nrf51 microbit:

python tools\make.py -m NRF51_MICROBIT -t ARM -n RTOS_1
Build Options: debug-info
Building project basic (NRF51_MICROBIT, ARM)
Scan: basic
Scan: mbed
Scan: rtos
Scan: env
Compile: main.cpp
[Error] main.cpp@6,0:  #35: #error directive: [NOT_SUPPORTED] test not supported
[ERROR] "C:\Code\git_repo\github\mbed\libraries\tests\rtos\mbed\basic\main.cpp", line 6: Error:  #35: #error directive: [NOT_SUPPORTED] test not supported
C:\Code\git_repo\github\mbed\libraries\tests\rtos\mbed\basic\main.cpp: 0 warnings, 1 error

Exported project contains also microlib configuration (tested for NRF51 MICROBIT), and K64F (not containing single thread as its full stdlib python tools\make.py -m K64F -t ARM -n RTOS_1)

This PR assumes we got GCC and ARM toolchains and provide small std lib option. Not as it is uARM vs ARM.

@c1728p9 @sg- @pan-

Same as for GCC, if small-build is selected, select microlib library. This
is currently duplicated from microlib. I assume we deprecate microlib toolchain, thus
duplication should be removed.
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 21, 2016

Note: This can be considered breaking as it changes for targets with ARM toolchain support and defined small-build. We could add a warning to this addition, that would be printed on the command line, that this has changed >122 release, and now it is using microlib (if microlib flag is defined).

cc @bogdanm @screamerbg

@pan-
Copy link
Member

pan- commented Jul 21, 2016

Looks good to me but this can be a breaking change for some targets.
On the other hand, without this change the behavior is inconsistent between toolchains.
If an application is developed with ARMCC first and use threads then the same application will have runtime errors if compiled with GCC.

@@ -81,6 +81,34 @@ def __init__(self, target, options=None, notify=None, macros=None, silent=False,
self.ar = join(ARM_BIN, "armar")
self.elf2bin = join(ARM_BIN, "fromelf")

# Use latest gcc nanolib
Copy link
Contributor

Choose a reason for hiding this comment

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

comment is a copy paste error from gcc.py ?

@sg-
Copy link
Contributor

sg- commented Jul 21, 2016

@mbed-bot: TEST

HOST_OSES=windows
BUILD_TOOLCHAINS=GCC_ARM,ARM,IAR
TARGETS=K64F,NRF51_DK,NRF51_MICROBIT,NUCLEO_F411RE,KL25Z

@mbed-bot
Copy link

[Build 662]
FAILURE: Something went wrong when building and testing.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jul 22, 2016

Note: needs:work label is the same as DON'T MERGE YET , we can eliminate one of them. needs: work can be changed to red label.

Back to the issue, the builds fail because of inconsistency. To provide small build, we use microlib for ARM, but microlib requires different startup file and many of targets do not support it. To complete this fix, we might need to unify this.

An example for LPC1768 - diff between two startups (microbit and full std):

--- C:\Code\git_repo\github\mbed\hal\targets\cmsis\TARGET_NXP\TARGET_LPC176X\TOOLCHAIN_ARM_MICRO\startup_LPC17xx.S  2016-07-07 11:03:18.170000000 +0100
+++ C:\Code\git_repo\github\mbed\hal\targets\cmsis\TARGET_NXP\TARGET_LPC176X\TOOLCHAIN_ARM_STD\startup_LPC17xx.S    2016-07-22 09:29:37.019000000 +0100
@@ -19,25 +19,8 @@
 ; *
 ; *****************************************************************************/

-Stack_Size      EQU     0x00000400
-
-                AREA    STACK, NOINIT, READWRITE, ALIGN=3
-                EXPORT  __initial_sp
-
-Stack_Mem       SPACE   Stack_Size
 __initial_sp        EQU     0x10008000  ; Top of RAM from LPC1768

-
-Heap_Size       EQU     0x00000000
-
-                AREA    HEAP, NOINIT, READWRITE, ALIGN=3
-                EXPORT  __heap_base
-                EXPORT  __heap_limit
-
-__heap_base
-Heap_Mem        SPACE   Heap_Size
-__heap_limit
-
                 PRESERVE8
                 THUMB


@c1728p9 You might be already looking at this, let me know

@c1728p9
Copy link
Contributor

c1728p9 commented Jul 22, 2016

@0xc0170, unfortunately I haven't looked in depth at the difference uARM and ARM startup files. This is something I definitely want to address in the future though.

@sg-
Copy link
Contributor

sg- commented Aug 26, 2016

@0xc0170 is this a duplicate or already handled by 5197edc

@pan-
Copy link
Member

pan- commented Aug 26, 2016

@sg- It is not a duplicate and it is not handled by 5197edc

@sg-
Copy link
Contributor

sg- commented Aug 26, 2016

Great, in that case probably needs update based on the naming in 5197edc

@sg- sg- removed the do not merge label Aug 26, 2016
@sg-
Copy link
Contributor

sg- commented Sep 28, 2016

@0xc0170 anything left to get done here or is this still on hold?

@0xc0170
Copy link
Contributor Author

0xc0170 commented Oct 4, 2016

Tracking issue: #2902 . Closing this as it's old and should be reconsidered.

@0xc0170 0xc0170 closed this Oct 4, 2016
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.

5 participants