-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add support for the TASKING compiler family, and its MIL linking feature #12342
base: master
Are you sure you want to change the base?
Conversation
Can you squash your commits where appropriate? Git history isn't making sense. |
I'll wait for the checks to finish, after that I will try |
1 similar comment
I'll wait for the checks to finish, after that I will try |
57b5c26
to
242a2f5
Compare
@tristan957 Is it better now? |
Here are the commits I envision:
Should there be any other reason for more commits? |
9838cdf
to
2817c5f
Compare
Fair enough, done :) |
885a87b
to
7f3a06e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know nothing about those compilers, but here are some general comments.
mesonbuild/compilers/detect.py
Outdated
else: | ||
raise EnvironmentException('Failed to detect linker for TASKING VX-toolset compiler. Please update your cross file(s).') | ||
|
||
tasking_ver_match = re.search(r'v(\d+)\.(\d+)r(\d+) Build (\d+)', err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use [0-9]
instead of \d
mesonbuild/compilers/detect.py
Outdated
return cls( | ||
ccache, compiler, tasking_version, for_machine, is_cross, info, | ||
exe_wrap, full_version=full_version, linker=linker) | ||
# TODO add detection logic here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have left that there by mistake, I'll take that comment out.
tasking_buildtype_args: T.Dict[str, T.List[str]] = { | ||
'plain': [], | ||
'debug': ['-g3', '-O0'], | ||
'debugoptimized': ['-g3', '-O2'], | ||
'release': ['-O3'], | ||
'minsize': ['-O3', '--tradeoff=4'], | ||
'custom': [] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use optimization and debug args instead of buildtype. -g3
is already in debug args. Other options are for optimization.
mesonbuild/backend/ninjabackend.py
Outdated
rules += [f"{rule}{ext}" for rule in [self.get_compiler_rule_name('tasking_mil_compile', compiler.for_machine)] | ||
for ext in ['', '_RSP']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is overcomplicated. Please imbricated for loops instead. Not sure why you iterate through a one-item list.
The MIL thing is a bit problematic as it is unique to this compiler and thus makes it a notable maintenance burden (as we don't have access to said compiler at all). All other compilers that I know of do this by outputting plain .o files with extra data in them (this is how link-time optimization works in e.g. Clang et al). Is there any data on how effective it is compared to plain unity builds? That is, if people could get the same performance using builtin unity support rather than this very toolchain specific thingy, would we need to support MIL at all? |
Unfortunately, I do not have any performance data. At first I didn't want to implement the MIL linking for the very reason you mention, but I was unable to make unity build work for the project I was converting to Meson due to some conditional compilation pattern used in almost all files, which pull in a specified part generated headers. With MIL linking, this is not a problem, files are compiled in an identical manner until this MIL intermediate language part. Not sure how to approach the fact that the compiler is not available for use, the best that I could think of was writing a dummy CLI emulation of the compiler, so some tests could run at the very least. |
Thanks for the TASKING Compiler support PR, I successfully used it with TASKING v6.3r1 to build firmware for Infineon TriCore TC375. The output hex file was flashed and it runs successfully on target. |
Hi, Is it possible to share sample files or configurations required to set up a build directory for Tasking Tricore please? Thank you. The cross_build.txt file I am using is attached. I ran command : meson setup --cross-file cross_build.ini builddir But I am getting errors :
One other question, I don't understand why I am getting :
Is there something I missed in my configurations ? I am not expecting my Visual Studio installation to be used in this build. Thank you. |
This should work, however, I did not implement support for C++. Not sure why it wants VS, for good measure I would pass in --wipe when configuring.
|
Hi @gerioldman, Thank you for your reply. I will continue to look into this as my team is trying to switch to meson/ninja. We are on a gmake based build system currently. update :
regards, |
Hi @tsetsong |
Thank you @afahmy-ANGENG for your reply. I work on a Windows 10 machine. Here are repository details and how I get the error :
And I built and installed my meson like this :
My python installation is at C:\Program Files\Python311
My Ninja is at : According to your instructions, I have done the following from my project folder :
The output is the same :
|
Hi all, Update : The line :
or
Will not show if you have gcc installed. The section of meson code responsible for this is in Interpreter.py :
It is possible to follow the instructions to install MinGW here : This will allow the shutil.which('gcc') to be True, and the function will return with 'False'. However, this does not solve the original problem of the generation crash. I am still looking into this. |
Hi, Update : I am now able to generate my build folder. An additional modification is needed in my cross_build.txt file. The
entry needs to be specified as :
I will proceed with compilation and porting and will report any other findings here as well. Here is the stdout :
I have also attached the build setup log (meson-log.txt file for reference. Cross_build file : Command ran :
I notice in the meson-log.txt file that the archiver is used instead of the linker when detecting linker :
How come ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry it's taken me so long to look at this. In general this all looks good. There's some thigns I think we can clean up here, but otherwise this looks fine from a code point of view.
mesonbuild/backend/ninjabackend.py
Outdated
@@ -402,7 +405,7 @@ def write(self, outfile): | |||
outfile.write(line) | |||
|
|||
if use_rspfile: | |||
if self.rule.rspfile_quote_style is RSPFileSyntax.MSVC: | |||
if self.rule.rspfile_quote_style is RSPFileSyntax.MSVC or self.rule.rspfile_quote_style is RSPFileSyntax.TASKING: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, use the in {...}
syntax
mesonbuild/backend/ninjabackend.py
Outdated
try: | ||
key = OptionKey('b_tasking_mil_link') | ||
if key in target.compilers['c'].base_options and target.get_option(key): | ||
final_obj_list = self.generate_mil_link(target, obj_list) | ||
else: | ||
final_obj_list = obj_list | ||
except KeyError: | ||
final_obj_list = obj_list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll hit that exception when you don't have a C compiler, but no other cases it looks like. Can we handle that without a try/except
, since that's not unexpected at all?
mesonbuild/backend/ninjabackend.py
Outdated
try: | ||
key = OptionKey('b_tasking_mil_link') | ||
if key in target.compilers['c'].base_options and target.get_option(key) and src.endswith('.c'): | ||
if obj_basename.endswith('.o'): | ||
obj_basename = obj_basename[:-1] + 'mil' | ||
else: | ||
obj_basename = obj_basename[:-3] + 'mil' | ||
except KeyError: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like the same case, where the missing c
compiler is the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that the target might not contain the built-in option, so it end ups throwing a KeyError. I've added a check if the target even contains it. Do you know any better solution? I've tried modifying the get_option call to fall back to the base_option var in the compilers.py file, but that had some CI failures.
But yes, the C compiler not being present is another problem, I've added a check for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, main has the same CI failures, maybe the base_option fallback was not the culprit then.
if 'PCP' in err: | ||
return linkers.TaskingPCPStaticLinker(linker) | ||
if 'MCS' in err: | ||
return linkers.TaskingMCSStaticLinker(linker) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you actually want to fall through here and end up with the system linker, link.exe
or ar
?
That looks like a Meson bug, we have the same message for both the archiver and dynamic linker... I'll send a patch for that. edit: #12784 |
2f9c728
to
8603a50
Compare
We try really hard to avoid adding new options without a good reason (because otherwise you have 1000+ options in the blink of an eye). Since this is "sort of" a link time optimization, would it be possible to make the code use the |
# This is a bit clever, for mypy we pretend that these mixins descend from | ||
# Compiler, so we get all of the methods and attributes defined for us, but | ||
# for runtime we make them descend from object (which all classes normally | ||
# do). This gives up DRYer type checking, with no runtime impact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is a typo: gives up
should be gives us
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected it.
8603a50
to
99340f8
Compare
I think that makes sense, I'll try to do that. Do you thing that 'b_lto' fits for the static library part of the MIL linking? What I mean is that right now, tasking_mil_link is possible to set for static libraries. This has the effect of LTO inside the library, but not with other code parts. However, it is possible to create MIL libraries, which behave as static libs, but still benefit from LTO at the actual link stage. Maybe a combination of prelink kward and b_lto could be used? |
I've just learned this, but there is a free, non-commercial version of the compiler bundled inside the Aurix Development Studio by Infineon. The compiler cannot be used outside the IDE though, a Meson build would have to be setup inside Eclipse. I am open to create a debug project, so you can try around with the compiler if any issues arise. |
cc15591
to
02d07a7
Compare
6159668
to
ec98f09
Compare
What happens if you use both at the same time? Based on this description "LTO inside a static lib" is pretty much the same as prelinking whereas "LTO over mutliple libraries" is what Meson calls regular LTO. So hooking it up to those would make logical sense. |
There should be three possible results:
I think prelink should take priorioty, someone might set LTO for the whole project, but prelinking can only be set per target. Based on that: What do you think? |
ec98f09
to
f5b9f4e
Compare
@@ -25,6 +25,7 @@ | |||
from .. import mesonlib | |||
from .. import mlog | |||
from ..compilers import LANGUAGES_USING_LDFLAGS, detect | |||
from ..utils.universal import get_compiler_for_source |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
'get_compiler_for_source' may not be defined if module
mesonbuild.utils.universal
mesonbuild.backend.backends
definition
import
from .mixins.emscripten import EmscriptenMixin | ||
from .mixins.metrowerks import MetrowerksCompiler | ||
from .mixins.metrowerks import mwccarm_instruction_set_args, mwcceppc_instruction_set_args | ||
from .mixins.tasking import TaskingCompiler |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.compilers.mixins.tasking
mesonbuild.compilers.c
definition
import
'TaskingCompiler' may not be defined if module
mesonbuild.compilers.mixins.tasking
mesonbuild.compilers.c
definition
import
'TaskingCompiler' may not be defined if module
mesonbuild.compilers.mixins.tasking
mesonbuild.compilers.c
definition
import
'TaskingCompiler' may not be defined if module
mesonbuild.compilers.mixins.tasking
mesonbuild.compilers.c
definition
import
@@ -9,6 +9,7 @@ | |||
from ...mesonlib import MesonException | |||
|
|||
if T.TYPE_CHECKING: | |||
from ...build import BuildTarget |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.build
mesonbuild.compilers.mixins.apple
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.apple
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.apple
definition
import
from .gnu import gnu_optimization_args | ||
from ...mesonlib import Popen_safe | ||
from ...options import OptionKey | ||
from ...build import BuildTarget |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.build
mesonbuild.compilers.mixins.elbrus
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.elbrus
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.elbrus
definition
import
@@ -17,6 +17,7 @@ | |||
from ... import mesonlib | |||
from ... import mlog | |||
from ...options import OptionKey | |||
from ...build import BuildTarget |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.build
mesonbuild.compilers.mixins.gnu
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.gnu
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.gnu
definition
import
import typing as T | ||
|
||
from ...mesonlib import EnvironmentException, File | ||
from ...options import OptionKey |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.options
mesonbuild.compilers.mixins.tasking
definition
import
'OptionKey' may not be defined if module
mesonbuild.options
mesonbuild.compilers.mixins.tasking
definition
import
|
||
from ...mesonlib import EnvironmentException, File | ||
from ...options import OptionKey | ||
from ...build import BuildTarget, StaticLibrary |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
'BuildTarget' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
|
||
from ...mesonlib import EnvironmentException, File | ||
from ...options import OptionKey | ||
from ...build import BuildTarget, StaticLibrary |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
'StaticLibrary' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
'StaticLibrary' may not be defined if module
mesonbuild.build
mesonbuild.compilers.mixins.tasking
definition
import
from ...compilers import lang_suffixes | ||
|
||
if T.TYPE_CHECKING: | ||
from ...compilers.compilers import Compiler |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
'Compiler' may not be defined if module
mesonbuild.compilers.compilers
mesonbuild.compilers.mixins.tasking
definition
import
e174630
to
e965caa
Compare
e965caa
to
684110c
Compare
This PR adds support for the TASKING compiler family, and its MIL linking feature as mentioned in #11866.