pass on asm flags for uVision project #10424

studavekar · 2019-04-16T21:46:35Z

Description

This should fix Keil uVision + armc6 build failure issue #10423 with PR #10390 .

Currently, asm flags are not passed on Uvision exporter

Lines 593 to 594 in 0b7d9af

	self.flags['asm'].append("--cpu=%s" % asm_cpu)

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@bridadan @SenRamakri @kjbracey-arm

Release Notes

ciarmcom · 2019-04-16T23:00:32Z

@studavekar, thank you for your changes.
@bridadan @SenRamakri @kjbracey-arm @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2019-04-17T08:02:43Z

Was this tested with few diff targets and projects? I recall some issues with asm + flags in uvision (length issue or similar).

Aborted CI build (was triggered automatically), we need CI time for 5.12.2 jobs today.

mbed-ci · 2019-04-17T08:02:49Z

Test run: FAILED

Summary: 6 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR

0xc0170 · 2019-04-17T08:30:22Z

Was this tested with few diff targets and projects? I recall some issues with asm + flags in uvision (length issue or similar).

I've checked only ARMC5 , please check also ARMCC6 if this is the case.

For ARMC5, we add common to asm flags - this could be problematic. If you check asm command line options and armcc , they differ. --gnu for instance is only armcc flag, not asm. I would not copy common to asm in this case (or remove from common flags that are not common for asm/armcc).

studavekar · 2019-04-17T15:25:53Z

Was this tested with few diff targets and projects? I recall some issues with asm + flags in uvision (length issue or similar).

CI should catch it.

For ARMC5, we add common to asm flags - this could be problematic.

I currently don't see common flag added to asm flags, i am looking at flags['asm']

0xc0170 · 2019-04-18T08:17:56Z

ARMC6 seems not to do it, but ARMC5 yes

ARMC5:
https://github.com/ARMmbed/mbed-os/blob/master/tools/toolchains/arm.py#L102

ARMC6:
https://github.com/ARMmbed/mbed-os/blob/master/tools/toolchains/arm.py#L600

Thus I don't think ARCM5 will work (not all common flags are for asm).

studavekar · 2019-04-18T14:51:05Z

@0xc0170 self.asm and self.flags['asm'] are not same we are using only self.flags['asm'] . However, we may not need this change as @bridadan was working on updating the uVIsion template to handle no fpu case.

adbridge · 2019-04-25T11:49:48Z

@studavekar what is the status of this PR ? Is it actually still needed ?

bridadan · 2019-04-25T15:53:40Z

@studavekar I've been looking at this a bit more. There are quite a few flags that are being duplicated in uvision (not 100% sure if this duplication is already present on the master branch as well, need to look more). I'm still looking into this, just taking longer than I was hoping. @kjbracey-arm is also looking at this a bit in #10390

adbridge · 2019-05-17T09:09:43Z

@kjbracey-arm is this related to some of your own going work? If so would you be able to pick this up also ?

studavekar · 2019-06-26T16:51:36Z

closing this PR, due to lack of progress on PR and won't be able to allocate time for this. Feel free to reopen.

jeromecoutant · 2020-12-14T16:20:44Z

Hi

Feel free to reopen.

@0xc0170 could you re-open this one ?
We are currently working on the new STM32WL target which is a M4 without FPU.
Without this patch I couldn't compile with Keil, so it seems a mandatory patch.

@MarceloSalazar

kjbracey · 2020-12-14T17:06:20Z

Hmm, I missed this at the time. Not that familiar with exporters, or the Python relating to them, but appears conceptually valid. There have been other similar issues with exporters not passing on assembly flags.

kjbracey · 2020-12-14T17:11:31Z

Oh, one concern I vaguely recall is getting the information passed at a "high enough" level. Rather than just passing all the flags in the command line under "misc flags", the exporters want to set higher-level XML options, like "core type" - so extract and translate the known options. This wouldn't be doing that.

(The one time I looked at this was #11225 for setting the language standard)

0xc0170 · 2020-12-18T09:18:04Z

As old tools were frozen, exporters as well. I would close this as won't fix.

cc @ARMmbed/mbed-os-tools

jeromecoutant · 2020-12-18T09:21:02Z

I can't agree...

0xc0170 · 2020-12-18T09:24:40Z

Let @ARMmbed/mbed-os-tools to review

hugueskamba · 2020-12-31T13:05:30Z

tools/export/uvision/__init__.py

@@ -175,7 +175,7 @@ def format_flags(self):
            '--cpreproc --cpreproc_opts=-D__ASSERT_MSG,' +
            ",".join("-D{}".format(s) for s in
                     self.toolchain.get_symbols(for_asm=True)))
-        flags['asm_flags'] = asm_flag_string
+        flags['asm_flags'] = "{} {}".format(" ".join(self.toolchain.flags['asm']), asm_flag_string)


@rwalton-arm

Given that this change is in the uVision project generator it shouldn't affect the behaviour in the online build service. Meaning we shouldn't have to create another "tools-release" for the online compiler. However, as we've had this PR open since April 2019, let's wait a few more days for @Patater to return before we decide if we're going to accept it. We will need to evaluate if this actually does affect Mbed Studio or the online build service.

@Patater can you please make a decision on this?

NB: This is mandatory for the on going STM32WL target which is a M4 without FPU.
Without this patch I couldn't compile with Keil IDE. Even if export is no more mandatory for ARM, it still very appreciated from customers.
Thx

The Mbed Online IDE actually can export things, so it would affect the Online IDE.

Sorry, we can't risk breaking the online tooling right now. That's why the tools are frozen. Exporter support is important and we should work to support it from the CMake-based build system instead of merging this risky PR.

jeromecoutant · 2021-01-14T16:11:12Z

Hi
Any update ?
Thx

ciarmcom · 2021-01-20T12:51:56Z

This pull request has automatically been marked as stale because it has had no recent activity. @studavekar, please carry out any necessary work to get the changes merged. Thank you for your contributions.

pass on asm flags for uVision project

8953f83

ciarmcom requested review from bridadan, kjbracey, SenRamakri and a team April 16, 2019 23:00

ciarmcom added the needs: review label Apr 16, 2019

SenRamakri approved these changes Apr 17, 2019

View reviewed changes

adbridge added needs: work and removed needs: review labels Apr 26, 2019

studavekar closed this Jun 26, 2019

jeromecoutant mentioned this pull request Dec 14, 2020

Keil uVision + armc6 build fails for device with nofpu #10423

Closed

MarceloSalazar reopened this Dec 14, 2020

hugueskamba reviewed Dec 31, 2020

View reviewed changes

ludoch-stm pushed a commit to ludoch-stm/mbed-os that referenced this pull request Jan 8, 2021

Patch from ARMmbed#10424

7243d9a

ciarmcom added the stale Stale Pull Request label Jan 20, 2021

studavekar closed this Jan 20, 2021

mergify bot removed needs: work stale Stale Pull Request labels Jan 20, 2021

jeromecoutant mentioned this pull request Jan 22, 2021

Adding NUCLEO_WL55JC support JeanMarcR/mbed-os#1

Open

pass on asm flags for uVision project #10424

pass on asm flags for uVision project #10424

Uh oh!

Conversation

studavekar commented Apr 16, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Apr 16, 2019

Uh oh!

0xc0170 commented Apr 17, 2019

Uh oh!

mbed-ci commented Apr 17, 2019

Test run: FAILED

Uh oh!

0xc0170 commented Apr 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

studavekar commented Apr 17, 2019

Uh oh!

0xc0170 commented Apr 18, 2019

Uh oh!

studavekar commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented Apr 25, 2019

Uh oh!

bridadan commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented May 17, 2019

Uh oh!

studavekar commented Jun 26, 2019

Uh oh!

jeromecoutant commented Dec 14, 2020

Uh oh!

kjbracey commented Dec 14, 2020

Uh oh!

kjbracey commented Dec 14, 2020

Uh oh!

0xc0170 commented Dec 18, 2020

Uh oh!

jeromecoutant commented Dec 18, 2020

Uh oh!

0xc0170 commented Dec 18, 2020

Uh oh!

hugueskamba Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

rwalton-arm Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

hugueskamba Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

jeromecoutant Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

Patater Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

jeromecoutant commented Jan 14, 2021

Uh oh!

ciarmcom commented Jan 20, 2021

Uh oh!

Uh oh!

0xc0170 commented Apr 17, 2019 •

edited

Loading

studavekar commented Apr 18, 2019 •

edited

Loading

bridadan commented Apr 25, 2019 •

edited

Loading