Hi, Thanks for the submission. Some comments below on this patch, but otherwise it looks good. I hope to get to the other patches in the series soon.
I haven't followed all of the previous discussion, so some of these points might already have been discussed. Sorry in advance if so. xucheng...@loongson.cn writes: > diff --git a/contrib/gcc_update b/contrib/gcc_update > index 1cf15f9b3c2..641ce164775 100755 > --- a/contrib/gcc_update > +++ b/contrib/gcc_update > @@ -86,6 +86,8 @@ gcc/config/arm/arm-tables.opt: gcc/config/arm/arm-cpus.in > gcc/config/arm/parsecp > gcc/config/c6x/c6x-tables.opt: gcc/config/c6x/c6x-isas.def > gcc/config/c6x/genopt.sh > gcc/config/c6x/c6x-sched.md: gcc/config/c6x/c6x-sched.md.in > gcc/config/c6x/gensched.sh > gcc/config/c6x/c6x-mult.md: gcc/config/c6x/c6x-mult.md.in > gcc/config/c6x/genmult.sh > +gcc/config/loongarch/loongarch-str.h: gcc/config/loongarch/genopts/genstr.sh > gcc/config/loongarch/genopts/loongarch-string > +gcc/config/loongarch/loongarch.opt: gcc/config/loongarch/genopts/genstr.sh > gcc/config/loongarch/genopts/loongarch.opt.in > gcc/config/m68k/m68k-tables.opt: gcc/config/m68k/m68k-devices.def > gcc/config/m68k/m68k-isas.def gcc/config/m68k/m68k-microarchs.def > gcc/config/m68k/genopt.sh > gcc/config/mips/mips-tables.opt: gcc/config/mips/mips-cpus.def > gcc/config/mips/genopt.sh > gcc/config/rs6000/rs6000-tables.opt: gcc/config/rs6000/rs6000-cpus.def > gcc/config/rs6000/genopt.sh > diff --git a/gcc/common/config/loongarch/loongarch-common.cc > b/gcc/common/config/loongarch/loongarch-common.cc > new file mode 100644 > index 00000000000..5bdfd2a30e1 > --- /dev/null > +++ b/gcc/common/config/loongarch/loongarch-common.cc > @@ -0,0 +1,73 @@ > +/* Common hooks for LoongArch. > + Copyright (C) 2021-2022 Free Software Foundation, Inc. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify > +it under the terms of the GNU General Public License as published by > +the Free Software Foundation; either version 3, or (at your option) > +any later version. > + > +GCC is distributed in the hope that it will be useful, > +but WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +GNU General Public License for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "tm.h" > +#include "common/common-target.h" > +#include "common/common-target-def.h" > +#include "opts.h" > +#include "flags.h" > +#include "diagnostic-core.h" > + > +#undef TARGET_OPTION_OPTIMIZATION_TABLE > +#define TARGET_OPTION_OPTIMIZATION_TABLE loongarch_option_optimization_table > + > +/* Set default optimization options. */ > +static const struct default_options loongarch_option_optimization_table[] = > +{ > + { OPT_LEVELS_ALL, OPT_fasynchronous_unwind_tables, NULL, 1 }, > + { OPT_LEVELS_NONE, 0, NULL, 0 } > +}; > + > +/* Implement TARGET_HANDLE_OPTION. */ > + > +static bool > +loongarch_handle_option (struct gcc_options *opts, > + struct gcc_options *opts_set ATTRIBUTE_UNUSED, > + const struct cl_decoded_option *decoded, > + location_t loc ATTRIBUTE_UNUSED) > +{ > + size_t code = decoded->opt_index; > + int value = decoded->value; > + > + switch (code) > + { > + case OPT_mmemcpy: > + if (value) > + { > + if (opts->x_optimize_size) > + opts->x_target_flags |= MASK_MEMCPY; > + } > + else > + opts->x_target_flags &= ~MASK_MEMCPY; > + return true; I think this will make the option order-dependent: -mmemcpy -Os could behave differently from -Os -mmemcpy. I'm also not sure it would trigger if the optimisation level is changed by the source code, e.g. using __attribute__((optimize)). If -mmemcpy is only supposed to have an effect when optimising for size, it would probably be better to have a preprocessor macro in loongson.h along the lines of: #define TARGET_MEMCPY (TARGET_RAW_MEMCPY && optimize_size) with s/Mask(MEMCPY)/Mask(RAW_MEMCPY)/ in the .opt file. > + > + default: > + return true; > + } > +} > + > +#undef TARGET_DEFAULT_TARGET_FLAGS > +#define TARGET_DEFAULT_TARGET_FLAGS MASK_CHECK_ZERO_DIV > +#undef TARGET_HANDLE_OPTION > +#define TARGET_HANDLE_OPTION loongarch_handle_option > + > +struct gcc_targetm_common targetm_common = TARGETM_COMMON_INITIALIZER; > […] > + # Inferring ISA-related default options from the ABI: pass 2 > + case ${with_abi}/${with_abiext} in > + lp64d/base) > + fpu_pattern="64" > + ;; > + lp64f/base) > + fpu_pattern="32|64" > + fpu_default="32" > + ;; > + lp64s/base) > + fpu_pattern="none|32|64" > + fpu_default="none" > + ;; > + *) > + echo "Unsupported ABI type ${with_abi}/${with_abiext}." > 1>&2 > + exit 1 > + ;; > + esac > + > […] > + ## Setting default value for with_fpu. > + if test x${with_fpu} == xnone; then > + with_fpu="0" > + fi I don't understand this. Won't it always trigger an error… > + > + case ${with_fpu} in > + "") > + if test x${fpu_default} != x; then > + with_fpu=${fpu_default} > + else > + with_fpu=${fpu_pattern} > + fi > + ;; > + > + *) > + if echo "${with_fpu}" | grep -E "^${fpu_pattern}$"; then > + : # OK > + else > + echo "${with_abi}/${with_abiext} ABI cannot be > implemented with" \ > + "--with-fpu=${with_fpu}." 1>&2 > + exit 1 …here? It looks like the original “none” would have been accepted for soft-float. > + fi > + ;; > + esac > + > + > + # Inferring default with_tune from with_arch: pass 1 > + case ${with_arch} in > + native) > + tune_pattern="*" > + tune_default="native" > + ;; > + loongarch64) > + tune_pattern="loongarch64 | la464" Does including the spaces work? The earlier patterns didn't have spaces, which seems more correct. > + tune_default="la464" > + ;; > + *) > + # By default, $with_tune == $with_arch > + tune_pattern="$with_arch" > + ;; > + esac > + > + ## Setting default value for with_tune. > + case ${with_tune} in > + "") > + if test x${tune_default} != x; then > + with_tune=${tune_default} > + else > + with_tune=${tune_pattern} > + fi > + ;; > + > + *) > + if echo "${with_tune}" | grep -E "^${tune_pattern}$"; > then > + : # OK > + else > + echo "Incompatible options: > --with-tune=${with_tune}" \ > + "and --with-arch=${with_arch}." 1>&2 > + exit 1 > + fi > + ;; > + esac > + > + > + # Perform final sanity checks. > + case ${with_arch} in > + loongarch64 | la464) ;; # OK, append here. > + native) > + if test x${host} != x${target}; then > + echo "--with-arch=native is illegal for > cross-compiler." 1>&2 > + exit 1 > + fi > + ;; > + "") > + echo "Please set a default value for \${with_arch}" \ > + "according to your target triplet \"${target}\"." > 1>&2 > + exit 1 > + ;; > + *) > + echo "Unknown arch in --with-arch=$with_arch" 1>&2 > + exit 1 > + ;; > + esac > + > + case ${with_abi} in > + lp64d | lp64f | lp64s) ;; # OK, append here. > + *) > + echo "Unsupported ABI given in --with-abi=$with_abi" > 1>&2 > + exit 1 > + ;; > + esac > + > + case ${with_abiext} in > + base) ;; # OK, append here. > + *) > + echo "Unsupported ABI extention type $with_abiext" 1>&2 > + exit 1 > + ;; > + esac > + > + case ${with_fpu} in > + none | 32 | 64) ;; # OK, append here. > + *) > + echo "Unknown fpu type in --with-fpu=$with_fpu" 1>&2 > + exit 1 > + ;; > + esac > + > + # Handle --with-multilib-list. > + if test x${with_multilib_list} == x \ > + || test x${with_multilib_list} == xno \ > + || test x${with_multilib_list} == xdefault \ > + || test x${enable_multilib} != xyes; then > + > + with_multilib_list="${with_abi}/${with_abiext}" > + fi > + > + # Check if the configured default ABI combination is included in > + # ${with_multilib_list}. > + loongarch_multilib_list_sane=no > + > + # This one goes to TM_MULTILIB_CONFIG, for use in t-linux. > + loongarch_multilib_list_make="" > + > + # This one goes to tm_defines, for use in loongarch-driver.c. > + loongarch_multilib_list_c="" > + > + for e in $(tr ',' ' ' <<< "${with_multilib_list}"); do > + e=($(tr '/' ' ' <<< "$e")) > + > + # Base ABI type > + case ${e[0]} in > + lp64d) loongarch_multilib_list_c+="ABI_BASE_LP64D,";; > + lp64f) loongarch_multilib_list_c+="ABI_BASE_LP64F,";; > + lp64s) loongarch_multilib_list_c+="ABI_BASE_LP64S,";; > + *) > + echo "Unknown base ABI \"${e[0]}\" in > --with-multilib-list." 1>&2 > + exit 1 > + ;; > + esac > + loongarch_multilib_list_make+="mabi=${e[0]}" > + > + # ABI extension type > + case ${e[1]} in > + "" | base) > + loongarch_multilib_list_make+="" > + loongarch_multilib_list_c+="ABI_EXT_BASE," > + e[1]="base" > + ;; > + *) > + echo "Unknown ABI extension \"${e[1]}\" in > --with-multilib-list." 1>&2 > + exit 1 > + ;; > + esac > + > + case ${e[2]} in > + "") ;; # OK > + *) > + echo "Unknown ABI in --with-multilib-list." 1>&2 > + exit 1 > + ;; > + esac > + > + if test x${with_abi} != x && test x${with_abiext} != x; > then > + if test x${e[0]} == x${with_abi} \ > + && test x${e[1]} == x${with_abiext}; then > + loongarch_multilib_list_sane=yes > + fi > + fi > + > + loongarch_multilib_list_make+="," > + done > + > + # Check if the default ABI combination is in the default list. > + if test x${loongarch_multilib_list_sane} == xno; then > + if test x${with_abiext} == xbase; then > + with_abiext="" > + else > + with_abiext="/${with_abiext}" > + fi > + > + echo "Default ABI combination > (${with_abi}${with_abiext})" \ > + "not found in --with-multilib-list." 1>&2 > + exit 1 > + fi > + > + # Remove the excessive appending comma. > + loongarch_multilib_list_c=${loongarch_multilib_list_c:0:-1} > + > loongarch_multilib_list_make=${loongarch_multilib_list_make:0:-1} I think ${…:…:…} is a bash-ism. And although bash is probably the system shell on the targets you care about, it's sometimes useful to test GCC with SHELL=/bin/dash in order to flush out unportable shell constructs. Maybe use sed 's/,$//' instead, even though it's much clunkier than your version. > + ;; > + > nds32*-*-*) > supported_defaults="arch cpu nds32_lib float fpu_config" > > @@ -5370,6 +5733,51 @@ case ${target} in > tmake_file="mips/t-mips $tmake_file" > ;; > > + loongarch*-*-*) > + # Export canonical triplet. > + tm_defines+=" LA_MULTIARCH_TRIPLET=${la_canonical_triplet}" Similarly here, += is a bashism. This unfortunately needs to be: tm_defines="${tm_defines} LA_MULTIARCH_TRIPLET=${la_canonical_triplet}" instead. Same for the rest of the patch. > + > + # Define macro __DISABLE_MULTILIB if --disable-multilib > + tm_defines+=" TM_MULTILIB_LIST=${loongarch_multilib_list_c}" > + if test x$enable_multilib == xyes; then > + TM_MULTILIB_CONFIG="${loongarch_multilib_list_make}" > + else > + tm_defines+=" __DISABLE_MULTILIB" > + fi __FOO macros are in the implementation namespace so GCC needs to avoid defining them itself. Maybe LA_DISABLE_MULTILIB instead? > […] > diff --git a/gcc/config/loongarch/genopts/genstr.sh > b/gcc/config/loongarch/genopts/genstr.sh > new file mode 100755 > index 00000000000..6c1211c9e92 > --- /dev/null > +++ b/gcc/config/loongarch/genopts/genstr.sh > @@ -0,0 +1,91 @@ > +#!/bin/sh > +# A simple script that generates loongarch-str.h and loongarch.opt > +# from genopt/loongarch-optstr. > +# > +# Copyright (C) 2021-2022 Free Software Foundation, Inc. > +# > +# This file is part of GCC. > +# > +# GCC is free software; you can redistribute it and/or modify it under > +# the terms of the GNU General Public License as published by the Free > +# Software Foundation; either version 3, or (at your option) any later > +# version. > +# > +# GCC is distributed in the hope that it will be useful, but WITHOUT > +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY > +# or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public > +# License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. > + > +cd "$(dirname "$0")" Similarly here we need to avoid bash's $(...). Would be worth trying the script with /bin/dash to see whether it works (i.e. produces the same output as bash). > diff --git a/gcc/config/loongarch/genopts/loongarch.opt.in > b/gcc/config/loongarch/genopts/loongarch.opt.in > new file mode 100644 > index 00000000000..01c8b3e5308 > --- /dev/null > +++ b/gcc/config/loongarch/genopts/loongarch.opt.in > @@ -0,0 +1,189 @@ > +; Generated by "genstr" from the template "loongarch.opt.in" > +; and definitions from "loongarch-strings". > +; > +; Copyright (C) 2021-2022 Free Software Foundation, Inc. > +; > +; This file is part of GCC. > +; > +; GCC is free software; you can redistribute it and/or modify it under > +; the terms of the GNU General Public License as published by the Free > +; Software Foundation; either version 3, or (at your option) any later > +; version. > +; > +; GCC is distributed in the hope that it will be useful, but WITHOUT > +; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY > +; or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public > +; License for more details. > +; > +; You should have received a copy of the GNU General Public License > +; along with GCC; see the file COPYING3. If not see > +; <http://www.gnu.org/licenses/>. > +; > + > +; Variables (macros) that should be exported by loongarch.opt: > +; la_opt_switches, > +; la_opt_abi_base, la_opt_abi_ext, > +; la_opt_cpu_arch, la_opt_cpu_tune, > +; la_opt_fpu, > +; la_cmodel. > + > +HeaderInclude > +config/loongarch/loongarch-opts.h > + > +HeaderInclude > +config/loongarch/loongarch-str.h > + > +Variable > +HOST_WIDE_INT la_opt_switches = 0 > + > +; ISA related options > +;; Base ISA > +Enum > +Name(isa_base) Type(int) > +Basic ISAs of LoongArch: > + > +EnumValue > +Enum(isa_base) String(@@STR_ISA_BASE_LA64V100@@) Value(ISA_BASE_LA64V100) > + > + > +;; ISA extensions / adjustments > +Enum > +Name(isa_ext_fpu) Type(int) > +FPU types of LoongArch: > + > +EnumValue > +Enum(isa_ext_fpu) String(@@STR_ISA_EXT_NOFPU@@) Value(ISA_EXT_NOFPU) > + > +EnumValue > +Enum(isa_ext_fpu) String(@@STR_ISA_EXT_FPU32@@) Value(ISA_EXT_FPU32) > + > +EnumValue > +Enum(isa_ext_fpu) String(@@STR_ISA_EXT_FPU64@@) Value(ISA_EXT_FPU64) > + > +m@@OPTSTR_ISA_EXT_FPU@@= > +Target RejectNegative Joined ToLower Enum(isa_ext_fpu) Var(la_opt_fpu) > Init(M_OPTION_NOT_SEEN) > +-m@@OPTSTR_ISA_EXT_FPU@@=FPU Generate code for the given FPU. For the record: I've not seen this kind of thing (with the @@ replacements) being done before, but I can see that it could be a way of preventing typos creeping in. It doesn't contradict the “global” GCC style and so IMO it's a perfectly valid choice for target maintainers to make. > […] > +mmemcpy > +Target Mask(MEMCPY) > +Don't optimize block moves. Going back to the above, if the intention is that this option only has an effect when optimising for size, the documentation ought to say that. > + > +mlra > +Target Var(loongarch_lra_flag) Init(1) Save > +Use LRA instead of reload. Please drop this option and only support LRA. No new code should use old reload, even as a default-off option. > + > +noasmopt > +Driver Do you need this? It looks like a MIPS hangover. Thanks, Richard
