NEW TOOLCHAIN: Add the ARMC6 compiler #4905

theotherjimmy · 2017-08-14T16:52:27Z

Description

This PR adds a new compiler to mbed OS: ARM Compiler 6 or armclang. This compiler
uses LLVM and CLang as the front end to an ARM proprietary optimization and code
generation layer. ARMC6 supports the new ARMv8M, ARMv8R and ARMv8A architectures.

Design notes

You may notice, when reviewing the code, that the ARMC6 toolchain class does not
inherit from the ARM toolchain class. This is intentional. While ARMC6 does use
the same linker (armlink) and assembler (armasm) as ARM Compiler 5, it is not
compatible.

The reason for this incompatibility are twofold:

armlink ~~and armasm~~ may invoke the C preprocessor by adding a shebang (#!) to
the top of their input files.
ARMC6 and ARMC5 differ in how you invoke the preprocessor:
- ARMC5: #! armcc -E
- ARMC6: #! armclang -E

When a toolchain inherits from another, each of their class names become
converted a TOOLCHAIN_<name> define and the resource scanner will pick up
files within directories of the same name. If ARMC6 were to inherit from the
ARM base class, it would pick up the arm assembler and linker files, which
contain a possibly incorrect preprocessor invocation. The correctness of the
preprocesssor invocation depends on if you have ARM Compiler 5 installed. This
is generally a Bad Thing. Therefore, I elected to force the semi-duplication of
these files to guarantee that the invocation is correct for each toolchain.
I have made a new partial Toolchain implementation for ARMASM, and created
a pair of

Current limitations

~~Presently, the tools are not creating or processing dependency files for ARMC6.~~
~~This can make development with it a headache. I consider this an essential~~
~~feature of a toolchain and will be updating this PR to include it.~~ Done.

Also, The output of ARMC6 is not filtered in any way to generate the short
version of each warning or error. This makes development with ARMC6 a little
verbose. I consider this a nonessential feature and will submit another PR to
include output processing unless someone convinces me otherwise.

Further, ARMC6 is not supported by the small build profile. This was an unintentional
omission, and I will correct it in this PR. Done.

This PR only adds K64F support for ARMC6. I think that other devices should be
supported with subsequent PRs, so I'm not going to add support in this PR.

There is no exporter for Uvision + ARMC6. I think this is also out of scope and
should come as a separate PR.

TODO

Write section detailing this new toolchain in the Handbook
Add ARMC6 to the CI
Run nightly test to verify my implementation of ARMC6
Process dependency information for ARM6
Add small profile
Small error/warning messages

c1728p9 · 2017-08-14T23:34:49Z

rtos/mbed_boot.c

@@ -232,7 +232,7 @@ osMutexAttr_t             singleton_mutex_attr;
 #if !defined(HEAP_START)
    #if defined(__ICCARM__)
        #error "Heap should already be defined for IAR"
-    #elif defined(__CC_ARM)
+    #elif defined(__CC_ARM) || defined (__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050)


You might consider wrapping defined (__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050) in parentheses as is done below.

c1728p9 · 2017-08-14T23:37:22Z

rtos/mbed_boot.c

@@ -400,7 +400,14 @@ void pre_main (void)
   With the RTOS there is not only one stack above the heap, there are multiple
   stacks and some of them are underneath the heap pointer.
 */
+#if defined (__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050)
+__asm(".global __use_two_region_memory\n\t");
+__asm(".global __use_no_semihosting\n\t");


Does the ARMCC reference manual recommend setting __use_two_region_memory and __use_no_semihosting using this syntax? Its a bit odd seeing inline assembly outside the body of a function.

Yes. https://developer.arm.com/docs/dui0742/latest/compiler-source-code-compatibility/language-extension-compatibility-pragmas

c1728p9 · 2017-08-14T23:39:23Z

rtos/rtx5/TARGET_CORTEX_M/rtx_lib.c

@@ -595,7 +595,6 @@ void *__user_perthread_libspace (void) {
 typedef void *mutex;

 // Initialize mutex
-__USED


Why was __USED removed for _mutex_initialize only and not the other functions like _mutex_free?

I think that has to be reverted, I remember linker was removing these symbols for some reason.

When that was in, it was conflicting with our define. We should look into why Used + Weak was not working correctly.

When I revert this change, I get the following linker error:

Error: L6200E: Symbol _mutex_initialize multiply defined (by rtx_lib.o and mbed_boot.o).

It looks like __USED beats __WEAK

@0xc0170 I think you fixed issues with the tests with ops (malloc?) not being thread safe due to these symbols being stripped out?

Looking through CMSIS RTX 5 source, it looks like we added the weak.

c1728p9 · 2017-08-14T23:40:08Z

targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/drivers/fsl_common.c

@@ -58,7 +58,7 @@ void __assert_func(const char *file, int line, const char *func, const char *fai
 void InstallIRQHandler(IRQn_Type irq, uint32_t irqHandler)
 {
 /* Addresses for VECTOR_TABLE and VECTOR_RAM come from the linker file */
-#if defined(__CC_ARM)
+#if defined(__CC_ARM) || defined (__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050)


parentheses

Is there any way we can simplify this?

@bulislaw Not that I'm aware of

c1728p9 · 2017-08-14T23:42:47Z

tools/memap.py

@@ -770,7 +770,7 @@ def parse(self, mapfile, toolchain):
                self.list_dir_obj(os.path.abspath(mapfile))

                if toolchain == "ARM" or toolchain == "ARM_STD" or\


nitpick - could you put this on one line with

if toolchain in ("ARM", "ARM_STD", "ARM_MICRO", "ARMC6"):

c1728p9 · 2017-08-14T23:43:48Z

tools/profiles/debug.json

+    "ARMC6": {
+        "common": ["-c", "--target=arm-arm-none-eabi", "-mthumb", "-g", "-O0"],
+        "asm": [],
+        "c": ["-D__ASSERT_MSG", "-fno-rtti", "-std=gnu99"],


Is -fno-rtti a valid C option? I thought rtti was only for C++ classes.

It does not seem to mind. I'll take it out

c1728p9 · 2017-08-14T23:45:10Z

tools/profiles/develop.json

@@ -23,6 +23,13 @@
        "cxx": ["--cpp", "--no_rtti", "--no_vla"],
        "ld": []
    },
+    "ARMC6": {
+        "common": ["-c", "--target=arm-arm-none-eabi", "-mthumb", "-g", "-O3", "-Os"],


Is it valid to have both -O3 and -Os?

That's how I read it. You can take a look here: https://developer.arm.com/docs/100066/latest/optimization/optimizing-for-code-size-or-performance

0xc0170 · 2017-08-15T07:23:35Z

👍 for the description, some of the info might be part of the commit message (I assume you will add more there once you rebase)

Add ARMC6 to the CI

@studavekar Please confirm as soon as ARMC6 is installed in our CI to test this PR (for those targets it supports at least , for now would be k64f)

theotherjimmy · 2017-08-15T14:01:38Z

@0xc0170 Detailed design description added to first commit message. squashed fix commits into Enable K64F commit.

theotherjimmy · 2017-08-15T14:03:28Z

@0xc0170 That second rebase was to re-flow the text of the commit message.

theotherjimmy · 2017-08-15T14:32:53Z

@c1728p9 I rebased again to address all of the comments that I could right now.

theotherjimmy · 2017-08-15T18:34:08Z

@c1728p9 Dependency parsing is up.

theotherjimmy · 2017-08-15T19:20:49Z

Some numbers:

mbed-os-example-blinky# mbed compile -t armc6 -m k64f --profile develop
[SNIP]
+-------------------+-------+-------+------+
| Module            | .text | .data | .bss |
+-------------------+-------+-------+------+
| [lib]/c_wu.l      | 12539 |    16 |  348 |
| [lib]/fz_wm.l     |    18 |     0 |    0 |
| [lib]/m_wm.l      |    48 |     0 |    0 |
| anon$$obj.o       |    32 |     0 | 1024 |
| lto-llvm-8f8bc0.o | 16418 |   200 | 6972 |
| mbed-os/rtos      |   184 |     0 |    0 |
| mbed-os/targets   |  1488 |     0 |    0 |
| Subtotals         | 30727 |   216 | 8344 |
+-------------------+-------+-------+------+
Total Static RAM memory (data + bss): 8560 bytes
Total Flash memory (text + data): 30943 bytes

mbed-os-example-blinky# mbed compile -t armc6 -m k64f --profile release
+-------------------+-------+-------+------+
| Module            | .text | .data | .bss |
+-------------------+-------+-------+------+
| [lib]/c_wu.l      |  9674 |    16 |  348 |
| [lib]/fz_wm.l     |    14 |     0 |    0 |
| [lib]/m_wm.l      |    48 |     0 |    0 |
| anon$$obj.o       |    32 |     0 | 1024 |
| lto-llvm-aebb6c.o | 15265 |   200 | 6956 |
| mbed-os/rtos      |   184 |     0 |    0 |
| mbed-os/targets   |  1488 |     0 |    0 |
| Subtotals         | 26705 |   216 | 8328 |
+-------------------+-------+-------+------+
Total Static RAM memory (data + bss): 8544 bytes
Total Flash memory (text + data): 26921 bytes

theotherjimmy · 2017-08-15T19:27:25Z

GCC for comparison

blinky# mbed compile -t gcc_arm -m k64f  --profile develop
+------------------+-------+-------+------+
| Module           | .text | .data | .bss |
+------------------+-------+-------+------+
| [fill]           |    62 |     4 | 2369 |
| [lib]/libc.a     | 22681 |  2472 |   56 |
| [lib]/libgcc.a   |  3090 |     0 |    0 |
| [lib]/libnosys.a |    32 |     0 |    0 |
| [lib]/misc       |   192 |     4 |   25 |
| main.o           |    56 |     0 |    4 |
| mbed-os/features |    42 |     0 |  184 |
| mbed-os/hal      |   570 |     0 |   24 |
| mbed-os/platform |  1226 |     4 |  270 |
| mbed-os/rtos     |  8439 |   180 | 5728 |
| mbed-os/targets  |  8027 |    12 |  384 |
| Subtotals        | 44417 |  2676 | 9044 |
+------------------+-------+-------+------+
Total Static RAM memory (data + bss): 11720 bytes
Total Flash memory (text + data): 47093 bytes

blinky# mbed compile -t gcc_arm -m k64f  --profile release
+------------------+-------+-------+------+
| Module           | .text | .data | .bss |
+------------------+-------+-------+------+
| [fill]           |    35 |     4 | 2217 |
| [lib]/libc.a     |  2456 |  2108 |   48 |
| [lib]/misc       |   192 |     4 |   25 |
| main.o           |    56 |     0 |    4 |
| mbed-os/features |    42 |     0 |  184 |
| mbed-os/hal      |   570 |     0 |   24 |
| mbed-os/platform |   720 |     4 |  270 |
| mbed-os/rtos     |  8439 |   180 | 5728 |
| mbed-os/targets  |  4930 |    12 |  384 |
| Subtotals        | 17440 |  2312 | 8884 |
+------------------+-------+-------+------+
Total Static RAM memory (data + bss): 11196 bytes
Total Flash memory (text + data): 19752 bytes

MarceloSalazar · 2017-08-15T20:44:13Z

This is a bit suspicious:
GCC (release), Flash = 19752 bytes
ARMC6 (release), Flash = 26921 bytes

Is the release profile optimizing for RAM at the expense of increasing Flash?

theotherjimmy · 2017-08-17T18:11:42Z

Alrightly, I rebased, squashing like changes, and splitting unlike changes. Should still build (I have a super cool CI like thing running in the background of my rebase) and the commit history should make more sense now.

bulislaw · 2017-08-18T10:02:58Z

rtos/rtx5/TARGET_CORTEX_M/rtx_lib.c

@@ -595,7 +595,9 @@ void *__user_perthread_libspace (void) {
 typedef void *mutex;

 // Initialize mutex
+#if !defined(__ARMCC_VERSION) || __ARMCC_VERSION < 6010050


I've asked in ARM-software/CMSIS_5#213 (comment) whether CMSIS guys have some better way of doing that.

Thanks @bulislaw. It looks like they're staying until the CMSIS guys can find a better solution.

theotherjimmy · 2017-08-18T14:26:04Z

FYI, another rebase coming. I'm going to try REPLACING ARMC5 with ARMC6. I think it will yield a much simpler implementation.

While ARMC6 does use the same linker (armlink) as ARM Compiler 5, it is not compatible. The reason for this incompatibility are twofold: * armlink may invoke the C preprocessor by adding a shebang (`#!`) to the top of their input files. * ARMC6 and ARMC5 differ in how you invoke the preprocessor: * ARMC5: `#! armcc -E` * ARMC6: `#! armclang -E` This does not yet handle dependencies properly

theotherjimmy · 2017-08-24T18:35:09Z

Closing this version of the PR, as the alternate implementation in #4949 Is better in most ways.

theotherjimmy added the needs: review label Aug 14, 2017

theotherjimmy force-pushed the feature-armc6 branch from fdae641 to 6de4878 Compare August 14, 2017 16:55

theotherjimmy mentioned this pull request Aug 14, 2017

Add ARMC6 where appropriate ARMmbed/mbed-os-5-docs#205

Merged

c1728p9 reviewed Aug 14, 2017

View reviewed changes

theotherjimmy force-pushed the feature-armc6 branch from 6de4878 to d16491f Compare August 15, 2017 14:00

theotherjimmy force-pushed the feature-armc6 branch from d16491f to eba0012 Compare August 15, 2017 14:02

theotherjimmy force-pushed the feature-armc6 branch 4 times, most recently from f86bb08 to b25d713 Compare August 15, 2017 14:32

theotherjimmy force-pushed the feature-armc6 branch from 803354f to 2837f6f Compare August 15, 2017 19:02

theotherjimmy force-pushed the feature-armc6 branch 3 times, most recently from 9a69359 to ab3cfac Compare August 17, 2017 18:09

theotherjimmy force-pushed the feature-armc6 branch from ab3cfac to 4bbbbc8 Compare August 17, 2017 18:13

bulislaw reviewed Aug 18, 2017

View reviewed changes

theotherjimmy added 4 commits August 18, 2017 10:33

Parse deps the same for GCC_ARM and ARMC6

34d7f6c

Update mbed OS to handle ARMC6 requirements

3ef4650

Enable Compiling K64F with ARMC6

ae8e7df

theotherjimmy force-pushed the feature-armc6 branch from 4bbbbc8 to ae8e7df Compare August 18, 2017 17:01

theotherjimmy added the type: duplicate label Aug 21, 2017

theotherjimmy mentioned this pull request Aug 21, 2017

NEW TOOLCHAIN: Add the ARMC6 Compiler #4949

Merged

7 tasks

theotherjimmy closed this Aug 24, 2017

theotherjimmy deleted the feature-armc6 branch August 24, 2017 18:35

theotherjimmy removed the needs: review label Aug 24, 2017

		@@ -770,7 +770,7 @@ def parse(self, mapfile, toolchain):
		self.list_dir_obj(os.path.abspath(mapfile))

		if toolchain == "ARM" or toolchain == "ARM_STD" or\

NEW TOOLCHAIN: Add the ARMC6 compiler #4905

NEW TOOLCHAIN: Add the ARMC6 compiler #4905

Uh oh!

Conversation

theotherjimmy commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Design notes

Current limitations

TODO

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 15, 2017

Uh oh!

MarceloSalazar commented Aug 15, 2017

Uh oh!

theotherjimmy commented Aug 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy commented Aug 24, 2017

Uh oh!

Uh oh!

theotherjimmy commented Aug 14, 2017 •

edited

Loading

theotherjimmy Aug 15, 2017 •

edited

Loading

theotherjimmy commented Aug 18, 2017 •

edited

Loading