Re: Adding support for the Xtensa architecture in crosstool-NG

["Yann E. MORIN" <yann dot morin dot 1998 at free dot fr>]

Chris, All,

Sorry for the delay. Here are my comments:

On 2013-07-24 17:09 -0700, Chris Zankel spake thusly:
> # HG changeset patch
> # User Chris Zankel <chris@zankel.net>
> # Date 1374703847 25200
> # Node ID 40864183a101b211d788ad94f569af324c74e56e
> # Parent  58ca31386bfbb48a522724dca79040efd54041c3
> xtensa: add support for the configurable Xtensa architecture.
> 
> The Xtensa processor architecture is a configurable, extensible,
> and synthesizable 32-bit RISC processor core. Processor and SOC vendors
> can select from various processor options and even create customized
> instructions in addition to a base ISA to tailor the processor for
> a particular application.
> 
> Because of the configurability, the build process requires one additional
> step for gcc, binutils, and gdb to update the default configuration.
> These configurations are packed into an 'overlay' tar image, and are
> simply untarred on top of the default configuration during the build.

[--SNIP--]

> diff -r 58ca31386bfb -r 40864183a101 scripts/build/arch/xtensa.sh
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/scripts/build/arch/xtensa.sh	Wed Jul 24 15:10:47 2013 -0700
> @@ -0,0 +1,75 @@
> +# Compute Xtensa-specific values
> +
> +CT_DoArchTupleValues() {
> +    # The architecture part of the tuple:
> +    CT_TARGET_ARCH="${CT_ARCH}${CT_ARCH_SUFFIX}"

Unneeded, that's the default.

> +    # The system part of the tuple:
> +    case "${CT_LIBC}" in
> +        *glibc)   CT_TARGET_SYS=gnu;;
> +        uClibc)   CT_TARGET_SYS=uclibc;;
> +    esac

Ditto.

> +}
> +
> +# This function updates the specified component (binutils, gcc, gdb, etc.)
> +# with the processor specific configuration.

Ouch! This one is a no-no.

Doing so means the extracted sources of the components can not be
re-used to build another toolchain (eg. another Xtensa toolchain with
another overlay, or for another architecture).

We already have a mechanism in place for the user to provide
"customised" sources, by way of the "custom location", see:
CUSTOM_LOCATION_ROOT_DIR

Basically, I do not like that an architecture has a hook to munge the
components' sources, or that do not have (complete) support in upstream.

> +CT_ConfigureXtensa() {
> +    local component="${1}"
> +    local version="${2}"
> +    local custom_overlay="${CT_ARCH_XTENSA_CUSTOM_OVERLAY_FILE}"
> +    local custom_location="${CT_ARCH_XTENSA_CUSTOM_OVERLAY_LOCATION}"
> +
> +    if [ -z "${CT_ARCH_XTENSA_CUSTOM_OVERLAY_FILE}" ]; then
> +        custom_overlay="xtensa-overlay.tar"
> +    fi
> +
> +    local full_file="${CT_TARBALLS_DIR}/${custom_overlay}"
> +    local basename="${component}-${version}"
> +    local ext
> +
> +    CT_TestAndAbort "${custom_overlay}: CT_CUSTOM_LOCATION_ROOT_DIR or CT_ARCH_XTENSA_CUSTOM_OVERLAY_LOCATION must be set." \
> +        -z "${CT_CUSTOM_LOCATION_ROOT_DIR}" -a -z "${custom_location}"
> +
> +    if [ -n "${CT_CUSTOM_LOCATION_ROOT_DIR}" \
> +         -a -z "${custom_location}" ]; then
> +             custom_location="${CT_CUSTOM_LOCATION_ROOT_DIR}"
> +    fi
> +
> +    if ! ext="$(CT_GetFileExtension "${basename}")"; then
> +        CT_DoLog WARN "'${basename}' not found in '${CT_TARBALLS_DIR}'"

There's no corresponding CT_GetFile. Who's responsible for downloading
that file?

> +        return 1
> +    fi
> +
> +    if [ -e "${CT_SRC_DIR}/.${basename}.configuring" ]; then
> +        CT_DoLog ERROR "The '${basename}' source were partially configured."
> +        CT_DoLog ERROR "Please remove first:"
> +        CT_DoLog ERROR " - the source dir for '${basename}', in '${CT_SRC_DIR}'"
> +        CT_DoLog ERROR " - the file '${CT_SRC_DIR}/.${basename}.extracted'"
> +        CT_DoLog ERROR " - the file '${CT_SRC_DIR}/.${basename}.patch'"
> +        CT_DoLog ERROR " - the file '${CT_SRC_DIR}/.${basename}.configuring'"
> +        CT_Abort
> +    fi
> +
> +    CT_DoLog EXTRA "Using '${custom_config}' from ${custom_location}"
> +    CT_DoExecLog DEBUG ln -sf "${custom_location}/${custom_config}" \
> +                              "${CT_TARBALLS_DIR}/${custom_config}"
> +
> +    CT_DoExecLog DEBUG touch "${CT_SRC_DIR}/.${basename}.configuring"
> +
> +    CT_Pushd "${CT_SRC_DIR}/${basename}"
> +
> +    tar_opts=( "--strip-components=1" )
> +    tar_opts+=( "-xv" )
> +
> +    case "${ext}" in
> +        .tar)           CT_DoExecLog FILE tar "${tar_opts[@]}" -f "${full_file}" "${component}";;
> +        .tar.gz|.tgz)   gzip -dc "${full_file}" | CT_DoExecLog FILE tar "${tar_opts[@]}" -f - "${component}";
> +        *)              CT_DoLog WARN "Don't know how to handle '${basename}${ext}': unknown extension"
> +                        return 1
> +                        ;;
> +    esac
> +
> +    CT_DoExecLog DEBUG touch "${CT_SRC_DIR}/.${basename}.configured"
> +    CT_DoExecLog DEBUG rm -f "${CT_SRC_DIR}/.${basename}.configuring"
> +
> +    CT_Popd
> +}

I'm not sure where to go from there.

Maybe just add "basic" Xtensa support that only requires what is already
in upstream gcc/binutils/gdb.

If the user requires a customised Xtensa configuration, then (s)he'd be
responsible for providing the mangled sources via the "custom location"
mechanism".

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

--
For unsubscribe information see http://sourceware.org/lists.html#faq