From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59087 invoked by alias); 20 Aug 2015 08:16:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 59068 invoked by uid 89); 20 Aug 2015 08:16:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 20 Aug 2015 08:16:00 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-2-zSj87LS2QnO4q8eB31D7Ig-1; Thu, 20 Aug 2015 09:15:54 +0100 Received: from e107456-lin.cambridge.arm.com ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 20 Aug 2015 09:15:54 +0100 From: James Greenhalgh To: gcc-patches@gcc.gnu.org Cc: marcus.shawcroft@arm.com, richard.earnshaw@arm.com, pinskia@gmail.com Subject: [AArch64] Break -mcpu tie between the compiler and assembler Date: Thu, 20 Aug 2015 08:16:00 -0000 Message-Id: <1440058549-24292-1-git-send-email-james.greenhalgh@arm.com> In-Reply-To: References: MIME-Version: 1.0 X-MC-Unique: zSj87LS2QnO4q8eB31D7Ig-1 Content-Type: multipart/mixed; boundary="------------1.9.1" X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg01163.txt.bz2 This is a multi-part message in MIME format. --------------1.9.1 Content-Type: text/plain; charset=UTF-8; format=fixed Content-Transfer-Encoding: quoted-printable Content-length: 12769 On Wed, Aug 19, 2015 at 04:48:11PM +0100, Andrew Pinski wrote: > On Wed, Aug 19, 2015 at 11:39 PM, James Greenhalgh > wrote: > > > > Hi, > > > > This patch has been sitting in my tree for a while - it comes in handy > > when trying out bootstrap or test with -mcpu values like -mcpu=3Dcortex= -a72 > > with a system assmebler which trails trunk binutils. > > > > Essentially, we rewrite -mcpu=3Dfoo to a -march flag providing the same > > architecture revision and set of optional architecture features. There > > is no reason we should ever need the assembler to see a CPU name, it > > should only be interested in the architecture variant. > > > > While we're there I've long found this function too fragile and hard > > to grok in C. So I've rewritten it in C++ to use std::string rather than > > raw C strings. Making this work with extension strings requires a slight > > refactor to the existing extension printing code to pull it across to > > somewhere common. > > > > Note that this also stops us from having to pick through a big.LITTLE > > target to find and separate the core names - we can just look up the > > architecture of the whole target and use that. > > > > The new function does leak the allocation of a C string to hold the > > result, but looking at gcc.c:getenv_spec_function and > > gcc.c:replace_extension_spec_func this is the usual thing to do. > > > > This has been through an aarch64-none-linux-gnu bootstrap and test run, > > configured with --with-cpu=3Dcortex-a72 , which my system assembler does > > not understand. > > > > Ok? > > > + modified string, which seems much worse! */ > + char *output =3D (char*) xmalloc (sizeof (*output) > + * (outstr.length () + 1)); > + strcpy (output, outstr.c_str ()); > > Why not just: > char *output =3D xstrdup (outstr.c_str ()); > > Or at least use XNEWVEC instead of xmalloc with a cast? Makes sense to me, patch updated along those lines. OK? Thanks, James --- 2015-08-19 James Greenhalgh * common/config/aarch64/aarch64-common.c (AARCH64_CPU_NAME_LENGTH): Delete. (aarch64_option_extension): New. (all_extensions): Likewise. (processor_name_to_arch): Likewise. (arch_to_arch_name): Likewise. (all_cores): New. (all_architectures): Likewise. (aarch64_get_extension_string_for_isa_flags): Likewise. (aarch64_rewrite_selected_cpu): Change to rewrite CPU names to architecture names. * config/aarch64/aarch64-protos.h (aarch64_get_extension_string_for_isa_flags): New. * config/aarch64/aarch64.c (aarch64_print_extension): Delete. (aarch64_option_print): Get the string to print from aarch64_get_extension_string_for_isa_flags. (aarch64_declare_function_name): Likewise. * config/aarch64/aarch64.h (BIG_LITTLE_SPEC): Rename to... (MCPU_TO_MARCH_SPEC): This. (ASM_CPU_SPEC): Use it. (BIG_LITTLE_SPEC_FUNCTIONS): Rename to... (MCPU_TO_MARCH_SPEC_FUNCTIONS): ...This. (EXTRA_SPEC_FUNCTIONS): Use it. diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config= /aarch64/aarch64-common.c index 726c625..07c6bba 100644 --- a/gcc/common/config/aarch64/aarch64-common.c +++ b/gcc/common/config/aarch64/aarch64-common.c @@ -27,7 +27,7 @@ #include "common/common-target-def.h" #include "opts.h" #include "flags.h" -#include "errors.h" +#include "diagnostic.h" =20 #ifdef TARGET_BIG_ENDIAN_DEFAULT #undef TARGET_DEFAULT_TARGET_FLAGS @@ -107,36 +107,134 @@ aarch64_handle_option (struct gcc_options *opts, =20 struct gcc_targetm_common targetm_common =3D TARGETM_COMMON_INITIALIZER; =20 -#define AARCH64_CPU_NAME_LENGTH 128 +/* An ISA extension in the co-processor and main instruction set space. */ +struct aarch64_option_extension +{ + const char *const name; + const unsigned long flags_on; + const unsigned long flags_off; +}; + +/* ISA extensions in AArch64. */ +static const struct aarch64_option_extension all_extensions[] =3D +{ +#define AARCH64_OPT_EXTENSION(NAME, FLAGS_ON, FLAGS_OFF, FEATURE_STRING) \ + {NAME, FLAGS_ON, FLAGS_OFF}, +#include "config/aarch64/aarch64-option-extensions.def" +#undef AARCH64_OPT_EXTENSION + {NULL, 0, 0} +}; + +struct processor_name_to_arch +{ + const std::string processor_name; + const enum aarch64_arch arch; + const unsigned long flags; +}; + +struct arch_to_arch_name +{ + const enum aarch64_arch arch; + const std::string arch_name; +}; + +/* Map processor names to the architecture revision they implement and + the default set of architectural feature flags they support. */ +static const struct processor_name_to_arch all_cores[] =3D +{ +#define AARCH64_CORE(NAME, X, IDENT, ARCH_IDENT, FLAGS, COSTS, IMP, PART) \ + {NAME, AARCH64_ARCH_##ARCH_IDENT, FLAGS}, +#include "config/aarch64/aarch64-cores.def" +#undef AARCH64_CORE + {"generic", AARCH64_ARCH_8A, AARCH64_FL_FOR_ARCH8}, + {"", aarch64_no_arch, 0} +}; + +/* Map architecture revisions to their string representation. */ +static const struct arch_to_arch_name all_architectures[] =3D +{ +#define AARCH64_ARCH(NAME, CORE, ARCH_IDENT, ARCH, FLAGS) \ + {AARCH64_ARCH_##ARCH_IDENT, NAME}, +#include "config/aarch64/aarch64-arches.def" +#undef AARCH64_ARCH + {aarch64_no_arch, ""} +}; + +/* Return a string representation of ISA_FLAGS. */ + +std::string +aarch64_get_extension_string_for_isa_flags (unsigned long isa_flags) +{ + const struct aarch64_option_extension *opt =3D NULL; + std::string outstr =3D ""; + + for (opt =3D all_extensions; opt->name !=3D NULL; opt++) + if ((isa_flags & opt->flags_on) =3D=3D opt->flags_on) + { + outstr +=3D "+"; + outstr +=3D opt->name; + } + return outstr; +} =20 -/* Truncate NAME at the first '.' character seen up to the first '+' - or return NAME unmodified. */ +/* Attempt to rewrite NAME, which has been passed on the command line + as a -mcpu option to an equivalent -march value. If we can do so, + return the new string, otherwise return an error. */ =20 const char * aarch64_rewrite_selected_cpu (const char *name) { - static char output_buf[AARCH64_CPU_NAME_LENGTH + 1] =3D {0}; - const char *bL_sep; - const char *feats; - size_t pref_size; - size_t feat_size; - - bL_sep =3D strchr (name, '.'); - if (!bL_sep) - return name; + std::string original_string (name); + std::string extensions; + std::string processor; + size_t extension_pos =3D original_string.find_first_of ('+'); =20 - feats =3D strchr (name, '+'); - feat_size =3D feats ? strnlen (feats, AARCH64_CPU_NAME_LENGTH) : 0; - pref_size =3D bL_sep - name; + /* Strip and save the extension string. */ + if (extension_pos !=3D std::string::npos) + { + processor =3D original_string.substr (0, extension_pos); + extensions =3D original_string.substr (extension_pos, + std::string::npos); + } + else + { + /* No extensions. */ + processor =3D original_string; + } =20 - if ((feat_size + pref_size) > AARCH64_CPU_NAME_LENGTH) - internal_error ("-mcpu string too large"); + const struct processor_name_to_arch* p_to_a; + for (p_to_a =3D all_cores; + p_to_a->arch !=3D aarch64_no_arch; + p_to_a++) + { + if (p_to_a->processor_name =3D=3D processor) + break; + } =20 - strncpy (output_buf, name, pref_size); - if (feats) - strncpy (output_buf + pref_size, feats, feat_size); + const struct arch_to_arch_name* a_to_an; + for (a_to_an =3D all_architectures; + a_to_an->arch !=3D aarch64_no_arch; + a_to_an++) + { + if (a_to_an->arch =3D=3D p_to_a->arch) + break; + } =20 - return output_buf; + /* We couldn't find that proceesor name, or the processor name we + found does not map to an architecture we understand. */ + if (p_to_a->arch =3D=3D aarch64_no_arch + || a_to_an->arch =3D=3D aarch64_no_arch) + fatal_error (input_location, "unknown value %qs for -mcpu", name); + + std::string outstr =3D a_to_an->arch_name + + aarch64_get_extension_string_for_isa_flags (p_to_a->flags) + + extensions; + + /* We are going to memory leak here, nobody elsewhere + in the callchain is going to clean up after us. The alternative is + to allocate a static buffer, and assert that it is big enough for our + modified string, which seems much worse! */ + return xstrdup (outstr.c_str ()); } =20 /* Called by the driver to rewrite a name passed to the -mcpu diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch= 64-protos.h index 0b09d49..06002d8 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -310,6 +310,7 @@ rtx aarch64_simd_gen_const_vector_dup (machine_mode, in= t); bool aarch64_simd_mem_operand_p (rtx); rtx aarch64_simd_vect_par_cnst_half (machine_mode, bool); rtx aarch64_tls_get_addr (void); +std::string aarch64_get_extension_string_for_isa_flags (unsigned long); tree aarch64_fold_builtin (tree, int, tree *, bool); unsigned aarch64_dbx_register_number (unsigned); unsigned aarch64_trampoline_size (void); diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index aa268ae..db316a0 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -7904,20 +7904,6 @@ initialize_aarch64_code_model (struct gcc_options *o= pts) aarch64_cmodel =3D opts->x_aarch64_cmodel_var; } =20 -/* Print to F the architecture features specified by ISA_FLAGS. */ - -static void -aarch64_print_extension (FILE *f, unsigned long isa_flags) -{ - const struct aarch64_option_extension *opt =3D NULL; - - for (opt =3D all_extensions; opt->name !=3D NULL; opt++) - if ((isa_flags & opt->flags_on) =3D=3D opt->flags_on) - asm_fprintf (f, "+%s", opt->name); - - asm_fprintf (f, "\n"); -} - /* Implement TARGET_OPTION_SAVE. */ =20 static void @@ -7950,10 +7936,12 @@ aarch64_option_print (FILE *file, int indent, struc= t cl_target_option *ptr) =3D aarch64_get_tune_cpu (ptr->x_explicit_tune_core); unsigned long isa_flags =3D ptr->x_aarch64_isa_flags; const struct processor *arch =3D aarch64_get_arch (ptr->x_explicit_arch); + std::string extension + =3D aarch64_get_extension_string_for_isa_flags (isa_flags); =20 fprintf (file, "%*sselected tune =3D %s\n", indent, "", cpu->name); - fprintf (file, "%*sselected arch =3D %s", indent, "", arch->name); - aarch64_print_extension (file, isa_flags); + fprintf (file, "%*sselected arch =3D %s%s\n", indent, "", + arch->name, extension.c_str ()); } =20 static GTY(()) tree aarch64_previous_fndecl; @@ -10677,8 +10665,11 @@ aarch64_declare_function_name (FILE *stream, const= char* name, const struct processor *this_arch =3D aarch64_get_arch (targ_options->x_explicit_arch); =20 - asm_fprintf (asm_out_file, "\t.arch %s", this_arch->name); - aarch64_print_extension (asm_out_file, targ_options->x_aarch64_isa_flags= ); + unsigned long isa_flags =3D targ_options->x_aarch64_isa_flags; + std::string extension + =3D aarch64_get_extension_string_for_isa_flags (isa_flags); + asm_fprintf (asm_out_file, "\t.arch %s%s\n", + this_arch->name, extension.c_str ()); =20 /* Print the cpu name we're tuning for in the comments, might be useful to readers of the generated asm. */ diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 1be78fc..1e5f5db 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -887,18 +887,18 @@ extern enum aarch64_code_model aarch64_cmodel; {"arch", "%{!march=3D*:%{!mcpu=3D*:-march=3D%(VALUE)}}" }, \ {"cpu", "%{!march=3D*:%{!mcpu=3D*:-mcpu=3D%(VALUE)}}" }, =20 -#define BIG_LITTLE_SPEC \ - " %{mcpu=3D*:-mcpu=3D%:rewrite_mcpu(%{mcpu=3D*:%*})}" +#define MCPU_TO_MARCH_SPEC \ + " %{mcpu=3D*:-march=3D%:rewrite_mcpu(%{mcpu=3D*:%*})}" =20 extern const char *aarch64_rewrite_mcpu (int argc, const char **argv); -#define BIG_LITTLE_CPU_SPEC_FUNCTIONS \ +#define MCPU_TO_MARCH_SPEC_FUNCTIONS \ { "rewrite_mcpu", aarch64_rewrite_mcpu }, =20 #if defined(__aarch64__) extern const char *host_detect_local_cpu (int argc, const char **argv); # define EXTRA_SPEC_FUNCTIONS \ { "local_cpu_detect", host_detect_local_cpu }, \ - BIG_LITTLE_CPU_SPEC_FUNCTIONS + MCPU_TO_MARCH_SPEC_FUNCTIONS =20 # define MCPU_MTUNE_NATIVE_SPECS \ " %{march=3Dnative:%