Move target specific macros to mbed_config.h

[sarahmarshy]

Fixes ARMmbed/mbed-os-example-client#61. Prevents uvision armasm command from growing too long with -D defines for preprocessing.

[bridadan]

/morph export-build

[sg-]

@bogdanm @screamerbg

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph export-build

[sarahmarshy]

Looks like I wasn't properly merged with master when making this patch. This commit:
c6c8d42#diff-1a2a8477037910723cb693931b3479ffR91

--preinclude for armasm as per infocenter is not valid. Maybe it is for a newer armasm, but uvision does not like it as assembler flag. I ran the export-build before and after this commit, and that is where it breaks.

[sarahmarshy]

/morph export-build

[mbed-bot]

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph export-build

[bogdanm]

I get what you're trying to do here, but conceptually, the config system doesn't know anything about toolchain macros (or about toolchains for that matter), it only deals with configuration data. While your fix would probably work, I find the idea of having toolchain macros listed in mbed_config.h confusing. Of course, if there's no other way to fix this, we'll end up doing this anyway.

[sarahmarshy]

@bogdanm Do you think an option might be to export another header file to be preincluded? Some of the device properties seem to be well placed in a configuration file. However, I am not intimately familiar with the configuration system, so I don't know what the conceptual boundaries are.

[sg-]

After reviewing this I think the path forward is:

• only core macros and target.macros will be passes to an assembler (mbed compile and mbed export)
• device_has and target / toolchain / features macros are emitted as -D (mbed compile and mbed export)
• configuration data is passed as --preinclude=mbed_config.h to C and C++ compilers (mbed compile and mbed export)

[bridadan]

@sg- The major implication I see for this is that if someone is writing their project in assembly (yikes) then they won't have access to the macros you mentioned. Though if they are writing their project in assembly, they're probably fine with this and they want all the control over what gets defined any way.

It may also be possible to still get the macros and write assembly by using a .c file with inline assembly? May be a viable workaround...

As long as this behavior is clearly documented in the config system's documentation in big, bold letters, this seems reasonable to me 👍

[sarahmarshy]

Moved here: #2377

[bogdanm]

Cc @AlessandroA. Are you guys happy with the solution proposed by @sg- above? To me, it looks like a preprocessed assembler source should see the same set of macros that a C/C++ source file sees, but maybe I'm just being paranoid.

[AlessandroA]

I agree with @bogdanm that the macros for preprocessed assembly files should be the same of the C/C++ files. It's not currently a requirement for uVisor, but I am confident these subtle differences in the build systems will eventually bite us.

Whenever is possible, I'd opt for consistency.

IIUC the limitation here is the armasm command length, right? If all toolchains support --preinclude or anything equivalent, we could keep all non-config definitions in a separate file and force-include them. In that sense, no symbol would be defined as a command line argument.

[sg-]

The problem is:

• assemblers dont support --preinclide (other than gcc)
• the various IDEs invocation environment or pattern cause the path length. Command line builds are OK

In general and historically:

• macros are macros, they should be everywhere
• device_has emits macros used for porting and turning on/off c hal implementations and C++ classes that call these implementations. Should be no regression or assembly usecase for these
• FEATURES / TOOLCHAIN / TARGET are used for scanning directories to build. Just so happens they also emit -D but TBH this was probably not by design, rather something that slipped in.
• config this is a whole new thing for MCUs. If I think about how the rest of the microcontroller world does this (right or wrong), its usually a header file that must be included in all translation units. Id go as far to guess that assemblers will barf on the majority of config data we try to pass it anyways (ie not legal defines).

Is there a specific case I've not considered? I think the above behavior is pretty consistent. Just don't use a hammer to thread a needle.

[AlessandroA]

@sg- At the moment FEATURE_UVISOR and TARGET_UVISOR_SUPPORTED are required to conditionally select code, both in ASM files and C/C++ files.

Commit 88564a9 removed those symbols for ASM compilation, introducing a regression in the uVisor app (see #2385).

FEATURE, TOOLCHAIN and TARGET are extensively used in mbed OS to conditionally select code. I don't see a reason for creating an exception for ASM files ("the tool doesn't allow it" is not good enough IMO).

If we agree that we need these symbols, then we can find other solutions (even a naive one — including a header at the beginning of the ASM files that need it). If you think we shouldn't use these symbols in code, then we should remove them altogether, not just from ASM files.

[bogdanm]

At the moment FEATURE_UVISOR and TARGET_UVISOR_SUPPORTED are required to conditionally select code, both in ASM files and C/C++ files.

This is what I thought originally that we're making too many assumptions about which macros are needed by the ASM code. The truth is that we can't know that in advance, so the only way to be sure that we're not messing things up is to make all macros available to ASM code, just like we do for C and C++ code. @sg-

[sg-]

macros are macros, TARGET_ TOOLCHAIN_ and FEATURE_ are not (even though we've misled you to think they are :) These are used for an inclusion method to compile. If you've relied on these its easy enough to macros_add FEATURE_UVISOR and TARGET_UVISOR_SUPPORTED