Skip to content

Fixed config-related options for the IAR assembler #2020

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.

Sign up for GitHub

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
Jun 27, 2016
Merged

Fixed config-related options for the IAR assembler #2020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tools/build_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ def build_project(src_path, build_path, target, toolchain_name,
# Load resources into the config system which might expand/modify resources based on config data
resources = config.load_resources(resources)

# Set the toolchain's config header with the config data
toolchain.set_config_header_content(config.get_config_data_header())
# Set the toolchain's configuration data
toolchain.set_config_data(config.get_config_data())

# Compile Sources
objects = toolchain.compile_sources(resources, build_path, resources.inc_dirs)
Expand Down Expand Up @@ -361,8 +361,8 @@ def build_library(src_paths, build_path, target, toolchain_name,
# Load resources into the config system which might expand/modify resources based on config data
resources = config.load_resources(resources)

# Set the toolchain's config header with the config data
toolchain.set_config_header_content(config.get_config_data_header())
# Set the toolchain's configuration data
toolchain.set_config_data(config.get_config_data())

# Copy headers, objects and static libraries - all files needed for static lib
toolchain.copy_files(resources.headers, build_path, resources=resources)
Expand Down
33 changes: 25 additions & 8 deletions tools/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ def get_config_data(self):
return all_params, macros

# Helper: verify if there are any required parameters without a value in 'params'
def _check_required_parameters(self, params):
@staticmethod
def _check_required_parameters(params):
for p in params.values():
if p.required and (p.value is None):
raise ConfigException("Required parameter '%s' defined by '%s' doesn't have a value" % (p.name, p.defined_by))
Expand All @@ -371,11 +372,18 @@ def parameters_to_macros(params):
def config_macros_to_macros(macros):
return [m.name for m in macros.values()]

# Return the configuration data converted to a list of C macros
# config - configuration data as (ConfigParam instances, ConfigMacro instances) tuple
# (as returned by get_config_data())
@staticmethod
def config_to_macros(config):
params, macros = config[0], config[1]
Config._check_required_parameters(params)
return Config.config_macros_to_macros(macros) + Config.parameters_to_macros(params)

# Return the configuration data converted to a list of C macros
def get_config_data_macros(self):
params, macros = self.get_config_data()
self._check_required_parameters(params)
return self.config_macros_to_macros(macros) + self.parameters_to_macros(params)
return self.config_to_macros(self.get_config_data())

# Returns any features in the configuration data
def get_features(self):
Expand Down Expand Up @@ -419,14 +427,16 @@ def load_resources(self, resources):

return resources


# Return the configuration data converted to the content of a C header file,
# meant to be included to a C/C++ file. The content is returned as a string.
# If 'fname' is given, the content is also written to the file called "fname".
# WARNING: if 'fname' names an existing file, that file will be overwritten!
def get_config_data_header(self, fname = None):
params, macros = self.get_config_data()
self._check_required_parameters(params)
# config - configuration data as (ConfigParam instances, ConfigMacro instances) tuple
# (as returned by get_config_data())
@staticmethod
def config_to_header(config, fname = None):
params, macros = config[0], config[1]
Config._check_required_parameters(params)
header_data = "// Automatically generated configuration file.\n"
header_data += "// DO NOT EDIT, content will be overwritten.\n\n"
header_data += "#ifndef __MBED_CONFIG_DATA__\n"
Expand Down Expand Up @@ -459,3 +469,10 @@ def get_config_data_header(self, fname = None):
with open(fname, "wt") as f:
f.write(header_data)
return header_data

# Return the configuration data converted to the content of a C header file,
# meant to be included to a C/C++ file. The content is returned as a string.
# If 'fname' is given, the content is also written to the file called "fname".
# WARNING: if 'fname' names an existing file, that file will be overwritten!
def get_config_data_header(self, fname = None):
return self.config_to_header(self.get_config_data(), fname)
20 changes: 11 additions & 9 deletions tools/toolchains/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from os.path import join, splitext, exists, relpath, dirname, basename, split, abspath
from inspect import getmro
from copy import deepcopy
from tools.config import Config

from multiprocessing import Pool, cpu_count
from tools.utils import run_cmd, mkdir, rel_path, ToolException, NotSupportedException, split_path
Expand Down Expand Up @@ -245,8 +246,8 @@ def __init__(self, target, options=None, notify=None, macros=None, silent=False,
# Labels generated from toolchain and target rules/features (used for selective build)
self.labels = None

# config_header_content will hold the content of the config header (if used)
self.config_header_content = None
# This will hold the configuration data (as returned by Config.get_config_data())
self.config_data = None

# Non-incremental compile
self.build_all = False
Expand Down Expand Up @@ -892,26 +893,27 @@ def mem_stats(self, map):
map_csv = splitext(map)[0] + "_map.csv"
memap.generate_output('csv-ci', map_csv)

# "Prefix headers" are automatically included by the compiler at the beginning of
# each source file. They are used to provide configuration data.
# header_content - the content of the config header file.
def set_config_header_content(self, header_content):
self.config_header_content = header_content
# Set the configuration data
def set_config_data(self, config_data):
self.config_data = config_data

# Return the location of the config header. This function will create the config
# header first if needed. The header will be written in a file called "mbed_conf.h"
# located in the project's build directory.
# If config headers are not used (self.config_header_content is None), the function
# returns None
def get_config_header(self):
if self.config_header_content is None:
if self.config_data is None:
return None
config_file = join(self.build_dir, "mbed_config.h")
if not exists(config_file):
with open(config_file, "wt") as f:
f.write(self.config_header_content)
f.write(Config.config_to_header(self.config_data))
return config_file

# Return the list of macros geenrated by the build system
def get_config_macros(self):
return Config.config_to_macros(self.config_data) if self.config_data else []

from tools.settings import ARM_BIN
from tools.settings import GCC_ARM_PATH, GCC_CR_PATH
Expand Down
15 changes: 10 additions & 5 deletions tools/toolchains/iar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,22 @@ def cc_extra(self, object):
base, _ = splitext(object)
return ["-l", base + '.s.txt']

def get_compile_options(self, defines, includes):
def get_compile_options(self, defines, includes, for_asm=False):
opts = ['-D%s' % d for d in defines] + ['-f', self.get_inc_file(includes)]
config_header = self.get_config_header()
if config_header is not None:
opts = opts + ['--preinclude', config_header]
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()]
Copy link
Contributor

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@screamerbg screamerbg Jun 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bogdanm bogdanm Jun 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@screamerbg screamerbg Jun 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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).

else:
config_header = self.get_config_header()
if config_header is not None:
opts = opts + ['--preinclude', config_header]
return opts

@hook_tool
def assemble(self, source, object, includes):
# Build assemble command
cmd = self.asm + self.get_compile_options(self.get_symbols(), includes) + ["-o", object, source]
cmd = self.asm + self.get_compile_options(self.get_symbols(), includes, for_asm=True) + ["-o", object, source]

# Call cmdline hook
cmd = self.hook.get_cmdline_assembler(cmd)
Expand Down