Fixed config-related options for the IAR assembler #2020
Conversation
| if for_asm: | ||
| # The assembler doesn't support '--preinclude', so we need to add | ||
| # the macros directly | ||
| opts = opts + ['-D%s' % d for d in self.get_config_macros()] |
There was a problem hiding this comment.
@bogdanm This won't work with values that contain {} and "'. But in general I think we should filter these as assembler can't use strings anyway. E.g. this could be a filtered list of the macros that are defined and/or MACRO= statement. What do you think?
There was a problem hiding this comment.
You are 100% correct about {} and "", I've forgotten about that. Regarding filtering though, we can't just start to filter which macros we put on the command line. I think the right answer to this is a proper quoting of the macro values on the command line. But this requires a bit of thought, I'm not sure yet what's the best way to do it.
There was a problem hiding this comment.
What I meant is different. Unlike c/cpp, in asm files you can't do string comparisons and assignments etc. You cannot even set float values for macros in Keil / asm settings. Because of this we can filter out all macros that neither simple defines nor integer assignment.
There was a problem hiding this comment.
That might not be needed. Even if you set the value of a macro to an invalid value (like -DWRONG_MACRO={1,2,3,4}), you won't get an error until you try to use that macro. If the .S file doesn't try to use it at all, all is well.
There was a problem hiding this comment.
you won't get an error until you try to use that macro
A tool like uvision can't go further with assembly defines as shown above, therefore -DWRONG_MACRO={1,2,3,4} returns error that parsing failed (it's slightly cryptic error). That's what I experienced, therefore there's a fix in mbed codebase to remove floating point from TIMESTAMP for the macros in the uvision exporter.
There was a problem hiding this comment.
Ah OK. With #1975 in place, this won't be a problem for uVision anyway (note that this change applies only to IAR). We need to figure out how the other IDEs (like IAR) deal with this.
There was a problem hiding this comment.
Again, if you open Keil uVision, go to "Options for Target...", go to ASM tab and add a macro like MBED_TIMESTAMP=123232412.123232 or MBED_CONFIG_NSAPI_ETC={0xc0} then all compiles will fail and ARMCC will complain that invalid macro is specified. Knowing that this is an issue both for ARMCC and IAR we could filter these.
@bogdanm
There was a problem hiding this comment.
@screamerbg: with #1975 in place, uVision is not going to be an issue. For IAR, I just opened Embedded Workbench and added a definition DUMMY_MACRO={1,2,3,4,5} to the C/C++ Copmiler/Preprocessor tab. EW didn't complain about that, it failed with a different error regarding duplicate object files (which is the same error that I get if I remove the definition of DUMMY_MACRO).
The IAR assembler doesn't accept '--preinclude', but it accepts -D. This commit changes the way the config-related macros are propagated to the IAR assembler to use '-D' instead of '--preinclude'. This is the only change related to functionality, the others are small, backward compatible changes to the config code to make passing arguments to the toolchain instances easier. Tested by compiled blinky with IAR, GCC_ARM and ARM for K64F.
bogdanm
force-pushed
the
fix_iar_asm_options
branch
from
1e57c0e to
b4e8cf6
Compare
|
CI build failed because of an invalid import of |
|
CI passed with the latest change. |
|
Tried this locally, looks good to me. |
|
@mbed-bot: TEST HOST_OSES=windows |
|
[Build 538] |
|
+1 |
The IAR assembler doesn't accept '--preinclude', but it accepts -D.
This commit changes the way the config-related macros are propagated
to the IAR assembler to use '-D' instead of '--preinclude'. This is
the only change related to functionality, the others are small,
backward compatible changes to the config code to make passing arguments
to the toolchain instances easier.
Tested by compiled blinky with IAR, GCC_ARM and ARM for K64F.