diff mbox

[ARM] Fix assembly comment syntax in -mprint-tune-info

Message ID 5899D3D1.5050302@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Feb. 7, 2017, 2:04 p.m. UTC
Hi all,

Currently, -mprint-tune-info gives an assembly file that cannot be assembled because the branch cost values
are not properly commented. For example:
         @.tune parameters
                 @constant_limit:        1
                 @max_insns_skipped:     5
                 @prefetch.num_slots:    0
                 @prefetch.l1_cache_size:        -1
                 @prefetch.l1_cache_line_size:   -1
                 @prefer_constant_pool:  0
                 @branch_cost:   (s:speed, p:predictable)
                                 s&p     cost
                                 00      1
                                 01      1
                                 10      4
                                 11      4
                 @prefer_ldrd_strd:      0
                 @logical_op_non_short_circuit:  [1,1]
                 @prefer_neon_for_64bits:        0
                 @disparage_flag_setting_t16_encodings:  0
                 @string_ops_prefer_neon:        1
                 @max_insns_inline_memset:       8
                 @fusible_ops:   3
                 @sched_autopref:        0


This patch fixes it in the obvious way by adding the comment character on the branch costs printout:
         @.tune parameters
                 @constant_limit:        1
                 @max_insns_skipped:     5
                 @prefetch.num_slots:    0
                 @prefetch.l1_cache_size:        -1
                 @prefetch.l1_cache_line_size:   -1
                 @prefer_constant_pool:  0
                 @branch_cost:   (s:speed, p:predictable)
                 @               s&p     cost
                 @               00      1
                 @               01      1
                 @               10      4
                 @               11      4
                 @prefer_ldrd_strd:      0
                 @logical_op_non_short_circuit:  [1,1]
                 @prefer_neon_for_64bits:        0
                 @disparage_flag_setting_t16_encodings:  0
                 @string_ops_prefer_neon:        1
                 @max_insns_inline_memset:       8
                 @fusible_ops:   3
                 @sched_autopref:        0

This assembles properly. Also, for pedantry's sake I've replaced the explicit '@' comment character
with ASM_COMMENT_START, which I believe is the technically right thing to do.

 From my testing this has been broken since GCC 5 when the option was introduced, so I guess this isn't a regression fix.
However, it is of minimal impact and quite obvious.

So ok for trunk?

Bootstrapped and tested on arm-none-linux-gnueabihf.

Thanks,
Kyrill


2016-02-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (arm_print_tune_info): Use ASM_COMMENT_START instead
     of explicit '@'.  Add missing assembly comment marker on branch costs
     printout.

Comments

Richard Earnshaw (lists) Feb. 13, 2017, 2:59 p.m. UTC | #1
On 07/02/17 14:04, Kyrill Tkachov wrote:
> Hi all,
> 
> Currently, -mprint-tune-info gives an assembly file that cannot be
> assembled because the branch cost values
> are not properly commented. For example:
>         @.tune parameters
>                 @constant_limit:        1
>                 @max_insns_skipped:     5
>                 @prefetch.num_slots:    0
>                 @prefetch.l1_cache_size:        -1
>                 @prefetch.l1_cache_line_size:   -1
>                 @prefer_constant_pool:  0
>                 @branch_cost:   (s:speed, p:predictable)
>                                 s&p     cost
>                                 00      1
>                                 01      1
>                                 10      4
>                                 11      4
>                 @prefer_ldrd_strd:      0
>                 @logical_op_non_short_circuit:  [1,1]
>                 @prefer_neon_for_64bits:        0
>                 @disparage_flag_setting_t16_encodings:  0
>                 @string_ops_prefer_neon:        1
>                 @max_insns_inline_memset:       8
>                 @fusible_ops:   3
>                 @sched_autopref:        0
> 
> 
> This patch fixes it in the obvious way by adding the comment character
> on the branch costs printout:
>         @.tune parameters
>                 @constant_limit:        1
>                 @max_insns_skipped:     5
>                 @prefetch.num_slots:    0
>                 @prefetch.l1_cache_size:        -1
>                 @prefetch.l1_cache_line_size:   -1
>                 @prefer_constant_pool:  0
>                 @branch_cost:   (s:speed, p:predictable)
>                 @               s&p     cost
>                 @               00      1
>                 @               01      1
>                 @               10      4
>                 @               11      4
>                 @prefer_ldrd_strd:      0
>                 @logical_op_non_short_circuit:  [1,1]
>                 @prefer_neon_for_64bits:        0
>                 @disparage_flag_setting_t16_encodings:  0
>                 @string_ops_prefer_neon:        1
>                 @max_insns_inline_memset:       8
>                 @fusible_ops:   3
>                 @sched_autopref:        0
> 
> This assembles properly. Also, for pedantry's sake I've replaced the
> explicit '@' comment character
> with ASM_COMMENT_START, which I believe is the technically right thing
> to do.
> 
> From my testing this has been broken since GCC 5 when the option was
> introduced, so I guess this isn't a regression fix.
> However, it is of minimal impact and quite obvious.
> 
> So ok for trunk?
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Thanks,
> Kyrill
> 
> 
> 2016-02-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/arm/arm.c (arm_print_tune_info): Use ASM_COMMENT_START instead
>     of explicit '@'.  Add missing assembly comment marker on branch costs
>     printout.
> 
> arm-print-tune.patch
> 

OK.

R.

> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index a5ca53d16bc756704c92f9d9de022b4fa6147fed..b7f7179d99ff211e6be518fdbbc4bdff312d6a07 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -25955,46 +25955,55 @@ arm_emit_eabi_attribute (const char *name, int num, int val)
>  void
>  arm_print_tune_info (void)
>  {
> -  asm_fprintf (asm_out_file, "\t@.tune parameters\n");
> -  asm_fprintf (asm_out_file, "\t\t@constant_limit:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t" ASM_COMMENT_START ".tune parameters\n");
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "constant_limit:\t%d\n",
>  	       current_tune->constant_limit);
> -  asm_fprintf (asm_out_file, "\t\t@max_insns_skipped:\t%d\n",
> -	       current_tune->max_insns_skipped);
> -  asm_fprintf (asm_out_file, "\t\t@prefetch.num_slots:\t%d\n",
> -	       current_tune->prefetch.num_slots);
> -  asm_fprintf (asm_out_file, "\t\t@prefetch.l1_cache_size:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "max_insns_skipped:\t%d\n", current_tune->max_insns_skipped);
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefetch.num_slots:\t%d\n", current_tune->prefetch.num_slots);
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefetch.l1_cache_size:\t%d\n",
>  	       current_tune->prefetch.l1_cache_size);
> -  asm_fprintf (asm_out_file, "\t\t@prefetch.l1_cache_line_size:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefetch.l1_cache_line_size:\t%d\n",
>  	       current_tune->prefetch.l1_cache_line_size);
> -  asm_fprintf (asm_out_file, "\t\t@prefer_constant_pool:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefer_constant_pool:\t%d\n",
>  	       (int) current_tune->prefer_constant_pool);
> -  asm_fprintf (asm_out_file, "\t\t@branch_cost:\t(s:speed, p:predictable)\n");
> -  asm_fprintf (asm_out_file, "\t\t\t\ts&p\tcost\n");
> -  asm_fprintf (asm_out_file, "\t\t\t\t00\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "branch_cost:\t(s:speed, p:predictable)\n");
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\ts&p\tcost\n");
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t00\t%d\n",
>  	       current_tune->branch_cost (false, false));
> -  asm_fprintf (asm_out_file, "\t\t\t\t01\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t01\t%d\n",
>  	       current_tune->branch_cost (false, true));
> -  asm_fprintf (asm_out_file, "\t\t\t\t10\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t10\t%d\n",
>  	       current_tune->branch_cost (true, false));
> -  asm_fprintf (asm_out_file, "\t\t\t\t11\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t11\t%d\n",
>  	       current_tune->branch_cost (true, true));
> -  asm_fprintf (asm_out_file, "\t\t@prefer_ldrd_strd:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefer_ldrd_strd:\t%d\n",
>  	       (int) current_tune->prefer_ldrd_strd);
> -  asm_fprintf (asm_out_file, "\t\t@logical_op_non_short_circuit:\t[%d,%d]\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "logical_op_non_short_circuit:\t[%d,%d]\n",
>  	       (int) current_tune->logical_op_non_short_circuit_thumb,
>  	       (int) current_tune->logical_op_non_short_circuit_arm);
> -  asm_fprintf (asm_out_file, "\t\t@prefer_neon_for_64bits:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "prefer_neon_for_64bits:\t%d\n",
>  	       (int) current_tune->prefer_neon_for_64bits);
> -  asm_fprintf (asm_out_file,
> -	       "\t\t@disparage_flag_setting_t16_encodings:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "disparage_flag_setting_t16_encodings:\t%d\n",
>  	       (int) current_tune->disparage_flag_setting_t16_encodings);
> -  asm_fprintf (asm_out_file, "\t\t@string_ops_prefer_neon:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "string_ops_prefer_neon:\t%d\n",
>  	       (int) current_tune->string_ops_prefer_neon);
> -  asm_fprintf (asm_out_file, "\t\t@max_insns_inline_memset:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
> +	       "max_insns_inline_memset:\t%d\n",
>  	       current_tune->max_insns_inline_memset);
> -  asm_fprintf (asm_out_file, "\t\t@fusible_ops:\t%u\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "fusible_ops:\t%u\n",
>  	       current_tune->fusible_ops);
> -  asm_fprintf (asm_out_file, "\t\t@sched_autopref:\t%d\n",
> +  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "sched_autopref:\t%d\n",
>  	       (int) current_tune->sched_autopref);
>  }
>  
>
diff mbox

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index a5ca53d16bc756704c92f9d9de022b4fa6147fed..b7f7179d99ff211e6be518fdbbc4bdff312d6a07 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -25955,46 +25955,55 @@  arm_emit_eabi_attribute (const char *name, int num, int val)
 void
 arm_print_tune_info (void)
 {
-  asm_fprintf (asm_out_file, "\t@.tune parameters\n");
-  asm_fprintf (asm_out_file, "\t\t@constant_limit:\t%d\n",
+  asm_fprintf (asm_out_file, "\t" ASM_COMMENT_START ".tune parameters\n");
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "constant_limit:\t%d\n",
 	       current_tune->constant_limit);
-  asm_fprintf (asm_out_file, "\t\t@max_insns_skipped:\t%d\n",
-	       current_tune->max_insns_skipped);
-  asm_fprintf (asm_out_file, "\t\t@prefetch.num_slots:\t%d\n",
-	       current_tune->prefetch.num_slots);
-  asm_fprintf (asm_out_file, "\t\t@prefetch.l1_cache_size:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "max_insns_skipped:\t%d\n", current_tune->max_insns_skipped);
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefetch.num_slots:\t%d\n", current_tune->prefetch.num_slots);
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefetch.l1_cache_size:\t%d\n",
 	       current_tune->prefetch.l1_cache_size);
-  asm_fprintf (asm_out_file, "\t\t@prefetch.l1_cache_line_size:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefetch.l1_cache_line_size:\t%d\n",
 	       current_tune->prefetch.l1_cache_line_size);
-  asm_fprintf (asm_out_file, "\t\t@prefer_constant_pool:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefer_constant_pool:\t%d\n",
 	       (int) current_tune->prefer_constant_pool);
-  asm_fprintf (asm_out_file, "\t\t@branch_cost:\t(s:speed, p:predictable)\n");
-  asm_fprintf (asm_out_file, "\t\t\t\ts&p\tcost\n");
-  asm_fprintf (asm_out_file, "\t\t\t\t00\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "branch_cost:\t(s:speed, p:predictable)\n");
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\ts&p\tcost\n");
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t00\t%d\n",
 	       current_tune->branch_cost (false, false));
-  asm_fprintf (asm_out_file, "\t\t\t\t01\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t01\t%d\n",
 	       current_tune->branch_cost (false, true));
-  asm_fprintf (asm_out_file, "\t\t\t\t10\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t10\t%d\n",
 	       current_tune->branch_cost (true, false));
-  asm_fprintf (asm_out_file, "\t\t\t\t11\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "\t\t11\t%d\n",
 	       current_tune->branch_cost (true, true));
-  asm_fprintf (asm_out_file, "\t\t@prefer_ldrd_strd:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefer_ldrd_strd:\t%d\n",
 	       (int) current_tune->prefer_ldrd_strd);
-  asm_fprintf (asm_out_file, "\t\t@logical_op_non_short_circuit:\t[%d,%d]\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "logical_op_non_short_circuit:\t[%d,%d]\n",
 	       (int) current_tune->logical_op_non_short_circuit_thumb,
 	       (int) current_tune->logical_op_non_short_circuit_arm);
-  asm_fprintf (asm_out_file, "\t\t@prefer_neon_for_64bits:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "prefer_neon_for_64bits:\t%d\n",
 	       (int) current_tune->prefer_neon_for_64bits);
-  asm_fprintf (asm_out_file,
-	       "\t\t@disparage_flag_setting_t16_encodings:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "disparage_flag_setting_t16_encodings:\t%d\n",
 	       (int) current_tune->disparage_flag_setting_t16_encodings);
-  asm_fprintf (asm_out_file, "\t\t@string_ops_prefer_neon:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "string_ops_prefer_neon:\t%d\n",
 	       (int) current_tune->string_ops_prefer_neon);
-  asm_fprintf (asm_out_file, "\t\t@max_insns_inline_memset:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START
+	       "max_insns_inline_memset:\t%d\n",
 	       current_tune->max_insns_inline_memset);
-  asm_fprintf (asm_out_file, "\t\t@fusible_ops:\t%u\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "fusible_ops:\t%u\n",
 	       current_tune->fusible_ops);
-  asm_fprintf (asm_out_file, "\t\t@sched_autopref:\t%d\n",
+  asm_fprintf (asm_out_file, "\t\t" ASM_COMMENT_START "sched_autopref:\t%d\n",
 	       (int) current_tune->sched_autopref);
 }