Skip to content

Disable Arm Compiler 5 and Use Arm Compiler 6 by default #5406

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

Merged
merged 7 commits into from
Nov 16, 2017

Conversation

theotherjimmy
Copy link
Contributor

No description provided.

@theotherjimmy
Copy link
Contributor Author

/morph build

@mbed-ci
Copy link

mbed-ci commented Oct 31, 2017

Build : FAILURE

Build number : 393
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@studavekar
Copy link
Contributor

@theotherjimmy ARMC6 is no more supported which was used to test ARMC6 ? Should be just use the default ARM now with this PR and moving forward ?

00:01:52.506 mbed test --compile -m ARCH_PRO -t ARMC6 -DMBED_HEAP_STATS_ENABLED=1 -DMBED_STACK_STATS_ENABLED=1 -DMBED_TRAP_ERRORS_ENABLED=1
00:01:52.850 [mbed] WARNING: Could not find mbed program in current path "/builds/ws/mbed-os-build-matrix/393/ARCH_PRO_ARMC6/sources/mbed-os".
00:01:52.850 [mbed] WARNING: You can fix this by calling "mbed new ." in the root of your program.
00:01:52.850 ---
00:01:53.546 usage: test.py [-h] [-m MCU] [-t TOOLCHAIN] [--color] [--cflags CFLAGS]
00:01:53.546                [--asmflags ASMFLAGS] [--ldflags LDFLAGS] [-c]
00:01:53.546                [--profile PROFILE] [--app-config APP_CONFIG] [-D MACROS]
00:01:53.546                [-j JOBS] [--source SOURCE_DIR] [--build BUILD_DIR] [-l]
00:01:53.546                [-p PATHS] [-f FORMAT] [--continue-on-build-fail] [-n NAMES]
00:01:53.546                [--test-config TEST_CONFIG] [--test-spec TEST_SPEC]
00:01:53.546                [--build-report-junit BUILD_REPORT_JUNIT]
00:01:53.546                [--build-data BUILD_DATA] [-v] [--stats-depth STATS_DEPTH]
00:01:53.546 test.py: error: argument -t/--tool: ARMC6 is not a supported toolchain. Supported toolchains are:
00:01:53.546 ARM,     GCC_ARM, IAR  

@mbed-ci
Copy link

mbed-ci commented Oct 31, 2017

Build : FAILURE

Build number : 398
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@theotherjimmy
Copy link
Contributor Author

Well crud. It still does not fit.

@theotherjimmy
Copy link
Contributor Author

I removed the last commit, as it introduced incompatibility with the STD library. We are just going to have to live with wchar and enum size differences.

@theotherjimmy
Copy link
Contributor Author

/morph build

@theotherjimmy
Copy link
Contributor Author

I removed the only offending test from most targets (it tests the toolchains)

@mbed-ci
Copy link

mbed-ci commented Nov 1, 2017

Build : FAILURE

Build number : 403
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@pan-
Copy link
Member

pan- commented Nov 2, 2017

I look at the C++ standard library size issue. The main offender is the vector class which pulls many things from the library: iostream, some exception support, and so on. On the other hand standard algorithms are fine and don't pull unrelated dependencies.

Looking at libc++ source code and our compilation options vector shouldn't pull all these dependencies. After some digging, I found out that the those symbols are pulled in when the vector base class std::__1::__vector_base_common<1> is imported from an implicit instantiation in libcpp_wnu.l(locale.cpp.o). More specifically, the code is pulled in when the member function __throw_length_error is used.

To avoid import of implicitly instantiated symbols, I've explicitly instantiated the class; templates instantiated implicitly generate weak symbols while template instantiated explicitly generates strong symbols therefore symbols resulting of an explicit instantiation will be picked up at link time.

The results are dramatically different:

  • Implicit instantiation:
+--------------+----------+-----------+------------+-------------+
| name         | target   | toolchain | static_ram | total_flash |
+--------------+----------+-----------+------------+-------------+
| stl_features | NRF52_DK | ARM       |      12576 |      129483 |
+--------------+----------+-----------+------------+-------------+

  • Explicit instantiation
+--------------+----------+-----------+------------+-------------+
| name         | target   | toolchain | static_ram | total_flash |
+--------------+----------+-----------+------------+-------------+
| stl_features | NRF52_DK | ARM       |       9109 |       49675 |
+--------------+----------+-----------+------------+-------------+

As surprising as it is, the code generated for std::__1::__vector_base_common<1>::__throw_length_error explicitly instantiated in the test source is similar to the one implicitly instantiated in libcpp; it is a simple call to abort.

Maybe there is something wrong with our link parameters which keep contents from locale.cpp.o. The call graph generated by the linker doesn't help, many symbols not referenced are kept in the final executable.

For the record, the problem can be reproduced with the following code:

#include <vector>

#define EXPLICIT_INSTANTIATION

/**
 * @brief provide access to a private class member function or member data
 * @details It is possible to explicitly instantiate a template class with a
 * private pointer to member. When such instantiation occur, a friend function
 * getMember is instantiated. This function will return the pointer to member
 * used at the instantiation.
 *
 * @tparam MemberAccessor Act as a key to resolve the correct overload of getMember
 * @tparam MemberType The type of the pointer to member that will be hijacked
 * @tparam Member The pointer to member hijacked
 */
template<typename MemberAccessor, typename MemberType, MemberType Member>
struct hijacked_member_accessor {
    friend MemberType getMember(MemberAccessor) {
        return Member;
    }
};

/**
 * @brief Provide an accessors to a pointer to member, even if this one is private
 *
 * @param ACCESSOR name of the accessor to create
 * @param MEMBER_TYPE type of the pointer to member
 * @param MEMBER the pointer to member hijacked
 */
#define HIJACK_MEMBER(ACCESSOR, MEMBER_TYPE, MEMBER) \
    struct ACCESSOR##_CLASS__ { \
            ACCESSOR##_CLASS__() { } \
            friend MEMBER_TYPE getMember(ACCESSOR##_CLASS__);\
        };\
    template struct hijacked_member_accessor<ACCESSOR##_CLASS__, MEMBER_TYPE, MEMBER>; \
    static MEMBER_TYPE const volatile ACCESSOR = getMember(ACCESSOR##_CLASS__())

typedef std::__1::__vector_base_common<1> vector_base_common;
typedef __attribute__((noreturn)) void (vector_base_common::*throw_length_error_type)() const;


// set to 1 to explicitly instante std::__1::__vector_base_common<1>
#ifdef EXPLICIT_INSTANTIATION
template class std::__1::__vector_base_common<1>;
#endif

// create an accessor to the member function pointer vector_base_common::__throw_length_error
// named throw_length_error_accessor
HIJACK_MEMBER(
    throw_length_error_accessor,
    throw_length_error_type,
    &vector_base_common::__throw_length_error
);

int main() {
    return 0;
}

The binary sizes are:

  • EXPLICIT_INSTANTIATION not defined:
+-----------------------+--------+-------+-------+
| Module                |  .text | .data |  .bss |
+-----------------------+--------+-------+-------+
| FileBase.o            |    118 |     0 |    56 |
| FilePath.o            |    158 |     0 |     0 |
| I2C.o                 |    116 |     0 |     0 |
| Mutex.o               |    154 |     0 |     0 |
| Thread.o              |     20 |     0 |     4 |
| TimerEvent.o          |    142 |     0 |     0 |
| [lib]/c_wu.l          |  21389 |    16 |   348 |
| [lib]/fz_wm.l         |   1296 |     0 |     0 |
| [lib]/libcpp_wnu.l    |  69703 |     0 |  3467 |
| [lib]/libcppabi_wnu.l |   2395 |     0 |     0 |
| [lib]/m_wm.l          |    870 |     0 |     0 |
| anon$$obj.o           |     32 |     0 |     0 |
| cmsis_nvic.o          |     16 |     0 |     0 |
| gpio_api.o            |    682 |     0 |   268 |
| irq_cm4f.o            |    184 |     0 |     0 |
| lp_ticker.o           |     94 |     0 |     0 |
| main.o                |     32 |     0 |     8 |
| mbed_assert.o         |     84 |     0 |     0 |
| mbed_board.o          |    370 |     0 |     0 |
| mbed_boot.o           |    440 |     0 |  4260 |
| mbed_critical.o       |     64 |     0 |     0 |
| mbed_error.o          |     56 |     0 |     1 |
| mbed_gpio.o           |    172 |     0 |     0 |
| mbed_lp_ticker_api.o  |     58 |     0 |    64 |
| mbed_retarget.o       |   1353 |     0 |   124 |
| mbed_rtx_handlers.o   |    657 |     0 |     0 |
| mbed_rtx_idle.o       |    658 |     4 |    52 |
| mbed_sleep_manager.o  |    232 |     0 |     2 |
| mbed_ticker_api.o     |    904 |     0 |     0 |
| mbed_us_ticker_api.o  |     58 |     0 |    64 |
| mbed_wait_api_rtos.o  |     90 |     0 |     0 |
| nordic_critical.o     |    330 |     0 |     5 |
| nrf_drv_common.o      |     72 |     0 |     0 |
| nrf_drv_gpiote.o      |   1366 |     0 |   105 |
| reloc_vector_table.o  |     40 |     0 |   216 |
| rt_OsEventObserver.o  |      0 |     0 |     4 |
| rtx_delay.o           |     76 |     0 |     0 |
| rtx_evr.o             |    114 |     0 |     0 |
| rtx_kernel.o          |   1355 |   176 |     0 |
| rtx_lib.o             |    364 |    24 |  2341 |
| rtx_memory.o          |    248 |     0 |     0 |
| rtx_mempool.o         |    272 |     0 |     0 |
| rtx_msgqueue.o        |   1830 |     0 |     0 |
| rtx_mutex.o           |    898 |     0 |     0 |
| rtx_system.o          |    404 |     0 |     0 |
| rtx_thread.o          |   2568 |     0 |     0 |
| rtx_timer.o           |    256 |     0 |     0 |
| serial_api.o          |   1178 |     0 |   136 |
| sleep.o               |    114 |     0 |     0 |
| softdevice_handler.o  |     12 |     0 |    13 |
| startup_nrf52832.o    |    264 |     0 |     0 |
| system_nrf52.o        |    620 |     4 |     0 |
| us_ticker.o           |    552 |     0 |     5 |
| Subtotals             | 115530 |   224 | 11543 |
+-----------------------+--------+-------+-------+
Total Static RAM memory (data + bss): 11767 bytes
Total Flash memory (text + data): 115754 bytes
  • EXPLICIT_INSTANTION defined:
+-----------------------+-------+-------+------+
| Module                | .text | .data | .bss |
+-----------------------+-------+-------+------+
| FileBase.o            |   118 |     0 |   56 |
| FilePath.o            |   158 |     0 |    0 |
| I2C.o                 |   116 |     0 |    0 |
| Mutex.o               |   154 |     0 |    0 |
| Thread.o              |    20 |     0 |    4 |
| TimerEvent.o          |   142 |     0 |    0 |
| [lib]/c_wu.l          | 11251 |    16 |  348 |
| [lib]/fz_wm.l         |    18 |     0 |    0 |
| [lib]/libcppabi_wnu.l |    44 |     0 |    0 |
| [lib]/m_wm.l          |    48 |     0 |    0 |
| anon$$obj.o           |    32 |     0 |    0 |
| cmsis_nvic.o          |    16 |     0 |    0 |
| gpio_api.o            |   682 |     0 |  268 |
| irq_cm4f.o            |   184 |     0 |    0 |
| lp_ticker.o           |    94 |     0 |    0 |
| main.o                |    36 |     0 |    8 |
| mbed_assert.o         |    84 |     0 |    0 |
| mbed_board.o          |   370 |     0 |    0 |
| mbed_boot.o           |   440 |     0 | 4260 |
| mbed_critical.o       |    64 |     0 |    0 |
| mbed_error.o          |    56 |     0 |    1 |
| mbed_gpio.o           |   172 |     0 |    0 |
| mbed_lp_ticker_api.o  |    58 |     0 |   64 |
| mbed_retarget.o       |   897 |     0 |  124 |
| mbed_rtx_handlers.o   |   657 |     0 |    0 |
| mbed_rtx_idle.o       |   658 |     4 |   52 |
| mbed_sleep_manager.o  |   232 |     0 |    2 |
| mbed_ticker_api.o     |   904 |     0 |    0 |
| mbed_us_ticker_api.o  |    58 |     0 |   64 |
| mbed_wait_api_rtos.o  |    90 |     0 |    0 |
| nordic_critical.o     |   330 |     0 |    5 |
| nrf_drv_common.o      |    72 |     0 |    0 |
| nrf_drv_gpiote.o      |  1366 |     0 |  105 |
| reloc_vector_table.o  |    40 |     0 |  216 |
| rt_OsEventObserver.o  |     0 |     0 |    4 |
| rtx_delay.o           |    76 |     0 |    0 |
| rtx_evr.o             |   114 |     0 |    0 |
| rtx_kernel.o          |  1355 |   176 |    0 |
| rtx_lib.o             |   364 |    24 | 2341 |
| rtx_memory.o          |   248 |     0 |    0 |
| rtx_mempool.o         |   272 |     0 |    0 |
| rtx_msgqueue.o        |  1830 |     0 |    0 |
| rtx_mutex.o           |   898 |     0 |    0 |
| rtx_system.o          |   404 |     0 |    0 |
| rtx_thread.o          |  2568 |     0 |    0 |
| rtx_timer.o           |   256 |     0 |    0 |
| serial_api.o          |  1136 |     0 |  136 |
| sleep.o               |   114 |     0 |    0 |
| softdevice_handler.o  |    12 |     0 |   13 |
| startup_nrf52832.o    |   264 |     0 |    0 |
| system_nrf52.o        |   620 |     4 |    0 |
| us_ticker.o           |   552 |     0 |    5 |
| Subtotals             | 30744 |   224 | 8076 |
+-----------------------+-------+-------+------+
Total Static RAM memory (data + bss): 8300 bytes
Total Flash memory (text + data): 30968 bytes

@theotherjimmy
Copy link
Contributor Author

/morph build

@theotherjimmy
Copy link
Contributor Author

Those two commits should get us testing in Austin CI

@theotherjimmy
Copy link
Contributor Author

now to get Travis working.

@mbed-ci
Copy link

mbed-ci commented Nov 7, 2017

Build : FAILURE

Build number : 449
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@theotherjimmy
Copy link
Contributor Author

I don't know what commit that was testing....

@theotherjimmy
Copy link
Contributor Author

Ah, looking at my reflog, it tested the pre-amend version of Add ARMC6 support to REALTEK post-binary hook

@theotherjimmy
Copy link
Contributor Author

/morph build

@theotherjimmy
Copy link
Contributor Author

@pan- I added the Explicit instantiation of the vector thingy to the stl_features test. Do you foresee any issues with that?

@mbed-ci
Copy link

mbed-ci commented Nov 7, 2017

Build : FAILURE

Build number : 450
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@mbed-ci
Copy link

mbed-ci commented Nov 7, 2017

Build : FAILURE

Build number : 451
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

@theotherjimmy
Copy link
Contributor Author

I just made the explicit instantination exclusive to armc6

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 9, 2017

/morph test

@mbed-ci
Copy link

mbed-ci commented Nov 9, 2017

I'm going to explain why these tests are superfluous because I don't
like removing tests. The asserts were all asserting something that does
not need to be true: that the toolchain's `name` attribute matched it's
key in the `TOOLCHIAN_CLASSES` dictionary. With that out of the way,
the `test_instantiation` test did not assert anything, and is therefore
not a test.
@theotherjimmy
Copy link
Contributor Author

/morph build

@theotherjimmy
Copy link
Contributor Author

Rebased to get commits from #5405

Arm compiler 6 optimized way too well, and adding 2 more nops did the
trick
@theotherjimmy
Copy link
Contributor Author

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 9, 2017

Build : SUCCESS

Build number : 480
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Nov 9, 2017

Build : SUCCESS

Build number : 481
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci
Copy link

mbed-ci commented Nov 9, 2017

@mbed-ci
Copy link

mbed-ci commented Nov 9, 2017

@studavekar
Copy link
Contributor

@theotherjimmy
Copy link
Contributor Author

Thanks. That's a persnickety one, and I'm having trouble reproducing it (My device is not operating well as a serial port).

@theotherjimmy
Copy link
Contributor Author

Alright, I debugged it, and it looks like the FPU is off, leading to a usage fault. I turned the FPU on in a debugger, and was able to get the platform to pass an arbitrary test.

@theotherjimmy
Copy link
Contributor Author

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 10, 2017

Build : SUCCESS

Build number : 495
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5406/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci
Copy link

mbed-ci commented Nov 10, 2017

@mbed-ci
Copy link

mbed-ci commented Nov 10, 2017

Test : SUCCESS

Build number : 313
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5406/313

@theotherjimmy theotherjimmy merged commit 5839c1b into ARMmbed:feature-armc6 Nov 16, 2017
@theotherjimmy theotherjimmy deleted the disable-armc5 branch November 16, 2017 20:19
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