| Message ID | 1387276843-21770-2-git-send-email-james.greenhalgh@arm.com |
|---|---|
| State | New |
| Headers | show |
On 17/12/13 10:40, James Greenhalgh wrote: > > Hi, > > This patch adds machinery to the driver to ensure that big.LITTLE > style tuning names are rewritten before they are passed to the > assembler. This reduces the coupling needed between GCC versions > and assembler versions. > > The rule is simple, we truncate the CPU name at the first '.' > character we see. > > Thus -mcpu=cortex-a15.cortex-a7 would be truncated to -mcpu=cortex-a15. > > Bootstrapped on a ChromeBook and checked for an arm-none-eabi and > an arm-none-linux-gnueabi build. > > Thanks, > James > > --- > gcc/ > > 2013-12-17 James Greenhalgh <james.greenhalgh@arm.com> > > * common/config/arm/arm-common.c (arm_rewrite_selected_cpu): New. > (arm_rewrite_mcpu): Likewise. > * config/arm/arm-protos.h (arm_rewrite_selected_cpu): New. > * config/arm/arm.h (BIG_LITTLE_SPEC): New. > (BIG_LITTLE_SPEC_FUNCTIONS): Likewise. > (EXTRA_SPEC_FUNCTIONS): Include BIG_LITTLE_SPEC_FUNCTIONS. > (ASM_CPU_SPEC): Include BIG_LITTLE_SPEC. > * config/arm/arm.c (arm_file_start): Rewrite arm_selecetd_cpu values. > OK. R.
Hi James,
This commit (SVN r206045) seems to have introduced a problem when
compiling multiple source files if a -mcpu option is also present on
the command line.
This can be reproduced in a arm-unknown-linux-gnueabihf build with any
source file which use floating point arguments/results. For example
===== t.c
float f(void) { return 1.0f;}
=====
Note that the example command line here specifies t.c twice, although
any two source files can be used. It is the second compilation which
triggers the error.
$ ~/tools/tools-arm-unknown-linux-gnueabihf-trunk/bin/arm-unknown-linux-gnueabihf-gcc
-mcpu=cortex-a15 -O2 -c t.c t.c
t.c: In function âfâ:
t.c:2:1: sorry, unimplemented: Thumb-1 hard-float VFP ABI
float f(void) { return 1.0f;}
^
Thanks in advance for taking a look
Charles
On 17 December 2013 11:53, Richard Earnshaw <rearnsha@arm.com> wrote:
> On 17/12/13 10:40, James Greenhalgh wrote:
>>
>> Hi,
>>
>> This patch adds machinery to the driver to ensure that big.LITTLE
>> style tuning names are rewritten before they are passed to the
>> assembler. This reduces the coupling needed between GCC versions
>> and assembler versions.
>>
>> The rule is simple, we truncate the CPU name at the first '.'
>> character we see.
>>
>> Thus -mcpu=cortex-a15.cortex-a7 would be truncated to -mcpu=cortex-a15.
>>
>> Bootstrapped on a ChromeBook and checked for an arm-none-eabi and
>> an arm-none-linux-gnueabi build.
>>
>> Thanks,
>> James
>>
>> ---
>> gcc/
>>
>> 2013-12-17 James Greenhalgh <james.greenhalgh@arm.com>
>>
>> * common/config/arm/arm-common.c (arm_rewrite_selected_cpu): New.
>> (arm_rewrite_mcpu): Likewise.
>> * config/arm/arm-protos.h (arm_rewrite_selected_cpu): New.
>> * config/arm/arm.h (BIG_LITTLE_SPEC): New.
>> (BIG_LITTLE_SPEC_FUNCTIONS): Likewise.
>> (EXTRA_SPEC_FUNCTIONS): Include BIG_LITTLE_SPEC_FUNCTIONS.
>> (ASM_CPU_SPEC): Include BIG_LITTLE_SPEC.
>> * config/arm/arm.c (arm_file_start): Rewrite arm_selecetd_cpu values.
>>
>
> OK.
>
> R.
>
>
On Wed, Jan 15, 2014 at 08:36:09PM +0000, Charles Baylis wrote: > Hi James, > > This commit (SVN r206045) seems to have introduced a problem when > compiling multiple source files if a -mcpu option is also present on > the command line. > > This can be reproduced in a arm-unknown-linux-gnueabihf build with any > source file which use floating point arguments/results. For example > > ===== t.c > float f(void) { return 1.0f;} > ===== > > Note that the example command line here specifies t.c twice, although > any two source files can be used. It is the second compilation which > triggers the error. > $ ~/tools/tools-arm-unknown-linux-gnueabihf-trunk/bin/arm-unknown-linux-gnueabihf-gcc > -mcpu=cortex-a15 -O2 -c t.c t.c > t.c: In function âfâ: > t.c:2:1: sorry, unimplemented: Thumb-1 hard-float VFP ABI > float f(void) { return 1.0f;} > ^ Hi Charles, I can't reproduce exactly the error you see, but I do see a bug which explains what is happening. The key can be seen by turning on -v. You will notice that COLLECT_GCC_OPTIONS for the first invocation of cc1 has a -mcpu value, but the second invocation does not. At a guess, your configuration is set with --with-mode=thumb, and probably has a default architecture of armv4. With the first invocation of cc1 the -mcpu overrides the default architecture and you get an armv7-a, hard-float, thumb mode binary. With the second invocation of cc1 there is no -mcpu and the compiler will try to build an armv4, hard-float, thumb mode binary - and fail with the message above. It should be the case that if we fix BIG_LITTLE_SPEC in arm.h such that the second invocation of cc1 also gets a -mcpu value, your issue will be resolved. I'll work on a fix. Thanks, James
On 16 January 2014 14:26, James Greenhalgh <james.greenhalgh@arm.com> wrote: > > At a guess, your configuration is set with --with-mode=thumb, > and probably has a default architecture of armv4. With the first > invocation of cc1 the -mcpu overrides the default architecture and Apologies, I should have included my configure line. I provide it here in case it helps. --target=arm-unknown-linux-gnueabihf --with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16 --with-float=hard --with-mode=thumb --enable-languages=c,c++ --with-sysroot=$SYSROOT Thanks Charles
diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c index c43a2ce..87f18ec 100644 --- a/gcc/common/config/arm/arm-common.c +++ b/gcc/common/config/arm/arm-common.c @@ -63,6 +63,41 @@ arm_except_unwind_info (struct gcc_options *opts) return UI_SJLJ; } +#define ARM_CPU_NAME_LENGTH 20 + +/* Truncate NAME at the first '.' character seen, or return + NAME unmodified. */ + +const char * +arm_rewrite_selected_cpu (const char *name) +{ + static char output_buf[ARM_CPU_NAME_LENGTH + 1] = {0}; + char *arg_pos; + + strncpy (output_buf, name, ARM_CPU_NAME_LENGTH); + arg_pos = strchr (output_buf, '.'); + + /* If we found a '.' truncate the entry at that point. */ + if (arg_pos) + *arg_pos = '\0'; + + return output_buf; +} + +/* Called by the driver to rewrite a name passed to the -mcpu + argument in preparation to be passed to the assembler. The + name will be in ARGV[0], ARGC should always be 1. */ + +const char * +arm_rewrite_mcpu (int argc, const char **argv) +{ + gcc_assert (argc == 1); + return arm_rewrite_selected_cpu (argv[0]); +} + +#undef ARM_CPU_NAME_LENGTH + + #undef TARGET_DEFAULT_TARGET_FLAGS #define TARGET_DEFAULT_TARGET_FLAGS (TARGET_DEFAULT | MASK_SCHED_PROLOG) diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index c5b16da..558f134 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -289,4 +289,7 @@ extern bool arm_autoinc_modes_ok_p (enum machine_mode, enum arm_auto_incmodes); extern void arm_emit_eabi_attribute (const char *, int, int); +/* Defined in gcc/common/config/arm-common.c. */ +extern const char *arm_rewrite_selected_cpu (const char *name); + #endif /* ! GCC_ARM_PROTOS_H */ diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 7027a26..a4ab6be 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -27527,7 +27527,11 @@ arm_file_start (void) else if (strncmp (arm_selected_cpu->name, "generic", 7) == 0) asm_fprintf (asm_out_file, "\t.arch %s\n", arm_selected_cpu->name + 8); else - asm_fprintf (asm_out_file, "\t.cpu %s\n", arm_selected_cpu->name); + { + const char* truncated_name + = arm_rewrite_selected_cpu (arm_selected_cpu->name); + asm_fprintf (asm_out_file, "\t.cpu %s\n", truncated_name); + } if (TARGET_SOFT_FLOAT) { diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 8b8b80e..6539ec6 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2343,16 +2343,25 @@ extern int making_const_table; instruction. */ #define MAX_LDM_STM_OPS 4 +#define BIG_LITTLE_SPEC \ + " %{mcpu=*:%<mcpu=* -mcpu=%:rewrite_mcpu(%{mcpu=*:%*})}" \ + +extern const char *arm_rewrite_mcpu (int argc, const char **argv); +#define BIG_LITTLE_CPU_SPEC_FUNCTIONS \ + { "rewrite_mcpu", arm_rewrite_mcpu }, + #define ASM_CPU_SPEC \ " %{mcpu=generic-*:-march=%*;" \ - " :%{mcpu=*:-mcpu=%*} %{march=*:-march=%*}}" + " :%{march=*:-march=%*}}" \ + BIG_LITTLE_SPEC /* -mcpu=native handling only makes sense with compiler running on an ARM chip. */ #if defined(__arm__) extern const char *host_detect_local_cpu (int argc, const char **argv); # define EXTRA_SPEC_FUNCTIONS \ - { "local_cpu_detect", host_detect_local_cpu }, + { "local_cpu_detect", host_detect_local_cpu }, \ + BIG_LITTLE_CPU_SPEC_FUNCTIONS # define MCPU_MTUNE_NATIVE_SPECS \ " %{march=native:%<march=native %:local_cpu_detect(arch)}" \ @@ -2360,6 +2369,7 @@ extern const char *host_detect_local_cpu (int argc, const char **argv); " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}" #else # define MCPU_MTUNE_NATIVE_SPECS "" +# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS #endif #define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS