arguments in TI C28x cross builds are incorrectly ordered and compiler args are missing from the linker command

EngJay commented 1 month ago

Describe the bug A clear and concise description of what the bug is.

There are three issues with the current support for TI code generation tools.

The compiler arguments are not included in the linker rule generated by the Ninja backend, so linking with the same arguments as compiling is impossible since the tools require that some of the options be included before the flag for linking, -z or --run_linker. TI's C28x code gen tools are weird, so the requirements of the arguments are substantially different from the general implementation of the Ninja backend for generating linker rules.
The order of $in and $LINK_ARGS in the linker rule generated by the Ninja backend for the TI C28x code gen tools is out of order from the specification of the CLI of the tools, which causes problems with some arguments.
The name of the TI code gen tools abstraction is imprecise because TI makes a wide variety of code gen tools with at least two different CLIs, only one of which is accommodated by the current implemetaton of the TICompiler and TIDynamicLinker classes.

Compiler arguments are not included when linking with TI C28x CGT

The TI C28x code gen tools require the compiler arguments to also be included when linking and they have to precede the flag that tells the tools to run the linker, -z or --run_linker. The Ninja rule does include $ARGS in the correct place but the variable is not populated. ~I have not been able to determine the cause of this, so I could use some help on it.~

It appears that the generate_link() method on the NinjaBackend does not add ARGS for any linking build rule, so a change would be needed there in order to include the compiler args in the correct place for the C28x CGT linker command.

The generated ninja build file shows the line for ARGS is not included for the linking build rule.

# Build rules for targets

build minimal-ti-c28x-cgt-f2837xd.out.p/src_main.c.o: c_COMPILER ../src/main.c
 DEPFILE = minimal-ti-c28x-cgt-f2837xd.out.p/src_main.c.o.d
 DEPFILE_UNQUOTED = minimal-ti-c28x-cgt-f2837xd.out.p/src_main.c.o.d
 ARGS = -I=minimal-ti-c28x-cgt-f2837xd.out.p -I=. -I=.. -I=/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/include --c99 -O2 '--define=RELEASE$ -DNDEBUG' -v28 -ml -mt --cla_support=cla1 --float_support=fpu32 --tmu_support=tmu0 --vcu_support=vcu2 --diag_suppress=10063 --diag_warning=225 --diag_wrap=off --display_error_number --abi=eabi --define=F2837xD --define=CPU1

build minimal-ti-c28x-cgt-f2837xd.out.p/src_F2837xD_CodeStartBranch.asm.o: c_COMPILER ../src/F2837xD_CodeStartBranch.asm
 DEPFILE = minimal-ti-c28x-cgt-f2837xd.out.p/src_F2837xD_CodeStartBranch.asm.o.d
 DEPFILE_UNQUOTED = minimal-ti-c28x-cgt-f2837xd.out.p/src_F2837xD_CodeStartBranch.asm.o.d
 ARGS = -I=minimal-ti-c28x-cgt-f2837xd.out.p -I=. -I=.. -I=/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/include --c99 -O2 '--define=RELEASE$ -DNDEBUG' -v28 -ml -mt --cla_support=cla1 --float_support=fpu32 --tmu_support=tmu0 --vcu_support=vcu2 --diag_suppress=10063 --diag_warning=225 --diag_wrap=off --display_error_number --abi=eabi --define=F2837xD --define=CPU1

build minimal-ti-c28x-cgt-f2837xd.out: c_LINKER minimal-ti-c28x-cgt-f2837xd.out.p/src_main.c.o minimal-ti-c28x-cgt-f2837xd.out.p/src_F2837xD_CodeStartBranch.asm.o
 LINK_ARGS = /repo/2837xD_RAM_lnk_cpu1.cmd -mminimal-ti-c28x-cgt-f2837xd.map --heap_size=0x200 --stack_size=0x3F8 --warn_sections -i/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/lib -i/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/include --reread_libs --define=RAM --diag_wrap=off --display_error_number --xml_link_info=minimal_ti_c28x_cgt_f2837xd_linkInfo.xml --rom_model -llibc.a

Order of linker args is incorrect for TI C28x CGT

The TI C28x family (C2000, C6000, etc.) of code generation tools expect a precise order of arguments with which the linker commands generated by Meson do not conform. This causes problems when including certain arguments.

The Ninja backend generates the linker commands for these TI code generation tools with the arguments in an incorrect order.

An example of the Ninja linker rule generated with the order of the $in and $LINK_ARGS reversed from the order specified by the TI CGT manuals.

[builddir]/build.ninja:

...

# Rules for linking.

rule c_LINKER
 command = cl2000 $ARGS -z --output_file=$out $in $LINK_ARGS
 description = Linking target $out

...

The order of the arguments is specified in the TI docs for the compilers.

C2000-CGT specification from section 2.2 of the manual:

cl2000 [options] [filenames] [--run_linker [link_options] object files]]

C6000-CGT specification from section 3.2 of the manual:

cl6x [options] [filenames] [--run_linker [link_options] object files]]

MSP430-CGT order of arguments for linking specification from section 2.2 of the manual.

cl430 [options] [filenames] [--run_linker [link_options] object files]]

This line in generate_dynamic_link_rules() in ninjabackend.py is the cause.

args = ['$ARGS'] + NinjaCommandArg.list(compiler.get_linker_output_args('$out'), Quoting.none) + ['$in', '$LINK_ARGS']

I believe an exception is needed in this function in order to generate the linker command with the arguments in the correct order for the TI C28x and C29x code generation tools.

TI code gen tools abstraction is imprecise

The implementation of the support for TI compilers is imprecise because TI makes a wide variety of code generation tools that include the C28x variety that is the subject of this PR but also GCC for TI devices CGT that uses the familiar GCC order of arguments. Due to that, support for the TI GCC compiler could not be derived from the TICompiler class defined in ti.py since the CLI is different.

To Reproduce Please include your meson.build files, preferably as a minimal toy example showing the issue. You may need to create simple source code files (don't include private/proprietary code).

A minimal example has been created as a self-contained example repo that only requires Docker to run. See the repo's README for details.

c2000.txt

# This file assumes that path to the Texas Instruments C20000 toolchain is added
# to the environment(PATH) variable, so that Meson can find
# cl2000 and ar2000 while building.
[binaries]
c = 'cl2000'
ar = 'ar2000'
strip = 'cl2000'

[host_machine]
system = 'bare metal'
cpu_family = 'c2000'
cpu = 'c28x'
endian = 'little'

[built-in options]
c_args = [
    '-v28',
    '-ml',
    '-mt']
c_link_args = []
cpp_args = []
cpp_link_args = []

meson.build

##
# @file meson.build
# @brief Top-level Meson module for minimal C28x build example for TI F28379D.
#
# @author Jason Scott <reachme@jasonpscott.com>
# @date 2024-09-13
#
# @copyright Copyright (c) 2024
#
project(
    'minimal-ti-c28x-cgt-f2837xd',
    ['c'],
    default_options: [
        'debug=false', # Defaults to release build.
        'warning_level=3',
        'optimization=2',
        # `build.*` options affect `native: true targets`,
        # plain options affect `native: false` targets.
        'c_std=c99',
        'build.c_std=c99',
    ],
    meson_version: '>=1.4.0',
    version: '0.1.0',
)

# TI C28x C2000 CGT tools location.
# 
ti_c28x_c2000_cgt_dir = '/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS'

#region FLAGS

# Build type.
#
build_is_debug = get_option('debug')
if build_is_debug
    # Specify build as debug and leave C assertions enabled.
    add_project_arguments('-DDEBUG', language: ['c'])
    add_project_arguments('-DDEBUG', language: ['c'], native: true)
else
    # Specify build as release and disable C assertions.
    add_project_arguments('-DRELEASE -DNDEBUG', language: ['c'])
    add_project_arguments('-DRELEASE -DNDEBUG', language: ['c'], native: true)
endif

c_compile_flags = [
    '--cla_support=cla1',
    '--float_support=fpu32',
    '--tmu_support=tmu0',
    '--vcu_support=vcu2',
    '--diag_suppress=10063',
    '--diag_warning=225',
    '--diag_wrap=off',
    '--display_error_number',
    '--abi=eabi', # Crucial flag that is not included in the linker rule by the Ninja backend.
    '--define=F2837xD',
    '--define=CPU1',
]

c_linker_flags = [
    '-mminimal-ti-c28x-cgt-f2837xd.map',
    '--heap_size=0x200',
    '--stack_size=0x3F8',
    '--warn_sections',
    '-i' + ti_c28x_c2000_cgt_dir / 'lib',
    '-i' + ti_c28x_c2000_cgt_dir / 'include',
    '--reread_libs',
    '--define=RAM',
    '--diag_wrap=off',
    '--display_error_number',
    '--xml_link_info=minimal_ti_c28x_cgt_f2837xd_linkInfo.xml',
    # '--entry_point=code_start',
    '--rom_model',
    '-llibc.a',
]

#endregion FLAGS

#region SOURCES

# The name of the excutable file to be created.
# 
app_executable_name = 'minimal-ti-c28x-cgt-f2837xd'

# Application and CGT include directories.
# 
app_include_dirs_arr = [
    ti_c28x_c2000_cgt_dir / 'include',
]

# Application source files.
# 
app_src_files = [
    'src' / 'main.c',
    'src' / 'F2837xD_CodeStartBranch.asm',
]

# Create objects to pass to executable target.
# 
all_app_include_dirs = include_directories(app_include_dirs_arr)
all_app_src_files = files(app_src_files)

# Linker script required for F2837xd CPU1 RAM.
# 
linker_script = meson.current_source_dir() / '2837xD_RAM_lnk_cpu1.cmd'

#endregion SOURCES

# Excutable target for the application.
# 
app_exe = executable(
    app_executable_name,
    all_app_src_files,
    include_directories: [all_app_include_dirs],
    c_args: [c_compile_flags],
    link_args: [linker_script, c_linker_flags],
)

Expected behavior A clear and concise description of what you expected to happen.

The expected behavior is to:

include the compiler arguments preceding the -z argument in the generated linker command.
include the input files after the output arguments.
include the linker arguments after the argument to run the linker,-z but before the output arguments.

The expected Ninja rule for the TI C28x CGT.

[builddir]/build.ninja:

...

# Rules for linking.

rule c_LINKER
 command = cl2000 $ARGS -z $LINK_ARGS --output_file=$out $in
 description = Linking target $out

...

A minimal example of the expected command with the expected output.

cl2000 -I=minimal-ti-c28x-cgt-f2837xd.out.p -I=. -I=.. -I=/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/include --c99 -O2 '--define=RELEASE -DNDEBUG' -v28 -ml -mt --cla_support=cla1 --float_support=fpu32 --tmu_support=tmu0 --vcu_support=vcu2 --diag_suppress=10063 --diag_warning=225 --diag_wrap=off --display_error_number --abi=eabi --define=F2837xD --define=CPU1 -z -mminimal-ti-c28x-cgt-f2837xd.map --heap_size=0x200 --stack_size=0x3F8 --warn_sections -i/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/lib -i/root/ti/cgt/ti-cgt-c2000_22.6.1.LTS/include --reread_libs --define=RAM --diag_wrap=off --display_error_number --xml_link_info=minimal_ti_c28x_cgt_f2837xd_linkInfo.xml --rom_model -llibc.a --output_file=minimal-ti-c28x-cgt-f2837xd.out minimal-ti-c28x-cgt-f2837xd.out.p/src_main.c.o minimal-ti-c28x-cgt-f2837xd.out.p/src_F2837xD_CodeStartBranch.asm.o /repo/2837xD_RAM_lnk_cpu1.cmd

system parameters

Is this a cross build or just a plain native build (for the same computer)?

A cross build using a c2000.txt cross file with the current LTS version of the TI C28x C2000 Code Generation Tools.

what operating system (e.g. MacOS Catalina, Windows 10, CentOS 8.0, Ubuntu 18.04, etc.)

Ubuntu 22.04 but in a Docker container running on macOS Ventura.

what Python version are you using e.g. 3.8.0

3.10.12

what meson --version

1.5.1

what ninja --version if it's a Ninja build

1.11.1.1

EngJay commented 1 month ago

I volunteer to address this - I think all that is needed is the addition of a conditional in the Ninja backend to change the order of the arguments when the compiler is TI C28x. And perhaps improving the naming or abstractions of the classes for the TI compilers and linkers.

EngJay commented 1 month ago

I fiddled around enough to produce a correct minimal build but additional issues are found when attempting to modify generate_dynamic_link_rules() and generate_link() to order the args correctly for TI C28x. After the args are added in the correct order, a new problem is caused by the static lib args since they are no longer last and come before the input files.

The TI compiler requires linked static libs to be absolutely last, after the input object files, and also requires the full library name, so -llibc.a or --library=libc.a, but the existing methods in Meson produce -lc when adding the dependency the proper Meson way and add it to the end of the linker args, which causes an error and undefined symbols.

What the TI compiler expects is libraries that end in .lib to be included in the list of input object files and static libs ending in .a to be included at the very end after the input object files.

What a correct Ninja linker rule looks like.

rule c_LINKER
 command = cl2000 $ARGS -z --output_file=$out $LINK_ARGS $in $TI_C28X_STATIC_LIBS
 description = Linking target $out

A TI C28 compiler-compliant command is then produced.

[1/1] cl2000 --c99 -O2 '--define=RELEASE -DNDEBUG' --cla_support=cla1 --float_support=fpu32 --tmu_support=tmu0 --vcu_support=vcu2 --diag_suppress=10063 --diag_warning=225 --diag_wrap=off --display_error_number --abi=eabi --define=F2837xD --define=CPU1 -v28 -ml -mt -z --output_file=src/example-i2c-ti-lp-f28379d-bme280.out /Users/ne10/embedded-hal-c/examples/ti/lp-f28379d/bme280/src/2837xD_RAM_lnk_cpu1.cmd /Users/ne10/embedded-hal-c/examples/ti/lp-f28379d/bme280/subprojects/c2000ware-core-sdk/driverlib/f2837xd/driverlib/ccs/Release/driverlib_eabi.lib -mmain.map --heap_size=0x200 --stack_size=0x3F8 --warn_sections -i/Applications/ti/ti-cgt-c2000_22.6.1.LTS/lib -i/Applications/ti/ti-cgt-c2000_22.6.1.LTS/include --reread_libs --define=RAM --diag_wrap=off --display_error_number --xml_link_info=example-i2c-ti-f28379d-bme280_linkInfo.xml --entry_point=code_start --rom_model src/example-i2c-ti-lp-f28379d-bme280.out.p/i2c_ex3_external_loopback.c.o src/example-i2c-ti-lp-f28379d-bme280.out.p/device_F2837xD_CodeStartBranch.asm.o src/example-i2c-ti-lp-f28379d-bme280.out.p/device_device.c.o -llibc.a
<Linking>

The TI C28x compilers have some really weird and specific requirements for how the commands have to be formatted that make trying to create them with the existing Meson implementation challenging but I think I have a way figured out with minimal changes. I'll open a PR once it's ironed out a little more.

EngJay commented 6 days ago

A fix for most of this was merged as #13681 and is included in the 1.7 milestone. The remaining issues with TI support will be addressed as subsequent issues.

mesonbuild / meson