Skip to content

Removed all references to __CC_ARM #12654

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 1 commit into from
Mar 20, 2020

Conversation

kbarm
Copy link

@kbarm kbarm commented Mar 19, 2020

Summary of changes

Removed ARMC5 reference from FEATURE_BLE.
ARM Compiler 5 is no longer supported in Mbed OS and is superseded by ARM Compiler 6

Impact of changes

Breaking change: The ARMC5 references are removed from platform as they have been deprecated in a previous Mbed OS release.

Migration actions required

Use Arm Compiler 6

Documentation

None.


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[x] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@evedon


Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Please fill in the template for the PR - introduce changes to all of us.

Also please add a reason for removing these in the commit message itself.

@mergify mergify bot added the needs: work label Mar 19, 2020
@kbarm kbarm force-pushed the feature-remove-cc-arm-macro branch from f3742b9 to 4465295 Compare March 19, 2020 15:03
@mergify mergify bot dismissed 0xc0170’s stale review March 19, 2020 15:04

Pull request has been modified.

@evedon
Copy link
Contributor

evedon commented Mar 19, 2020

This PR only removed ARMC5 references from FEATURE_BLE, not platform. Could you amend the title & description?

@ciarmcom ciarmcom requested review from evedon and a team March 19, 2020 16:00
@ciarmcom
Copy link
Member

@kbarm, thank you for your changes.
@evedon @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@evedon evedon left a comment

Choose a reason for hiding this comment

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

Looks good. Let's check CI.

@adbridge
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Mar 20, 2020

Test run: SUCCESS

Summary: 7 of 7 test jobs passed
Build number : 1
Build artifacts

Copy link
Member

@pan- pan- left a comment

Choose a reason for hiding this comment

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

Thanks @rajkan01 I believe the assembly part should be replaced with something compatible with ARM compiler 6 assembly. For some of the pragma, these are different too.

How do we differentiate GCC from ARMCC at compile time ?

#elif defined(__CC_ARM)
#define BLE_DEPRECATED_API_USE_BEGIN() \
_Pragma("push") \
_Pragma("diag_suppress 1361")
Copy link
Member

Choose a reason for hiding this comment

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

Could you use the clang pragma instead when the file is compiled with ARMC6 ?
Please have a look at: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0742b/chr1398848167694.html

}

#elif defined ( __ICCARM__ )
#if defined ( __ICCARM__ )
Copy link
Member

Choose a reason for hiding this comment

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

Why not provide the proper replacement for ARM compiler 6 assembler ?

ALIGN
}
#elif defined ( __GNUC__ )
#if defined ( __GNUC__ )
Copy link
Member

Choose a reason for hiding this comment

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

Is GCC assembly language compatible with ARMC6 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants