[PATCH v5] add -fprolog-pad=N,M option

Bernd Schmidt bschmidt@redhat.com
Mon Jan 23 16:45:00 GMT 2017


There's still a a few details that need addressing, and some questions I 
have. Also Ccing Jakub to have another pair of eyes on the name of the 
section - I don't know if we want some sort of .gnu.something name.

On 01/13/2017 01:19 PM, Torsten Duwe wrote:
>
> 2017-01-13	Torsten Duwe	<duwe@suse.de> :

First, some people seem to care, so I'll have to ask to please check the 
correct formatting of this line (spacing and all) by looking at existing 
entries. Also remove some of the blank lines in the ChangeLog, you could 
still group the doc entries separately from the code ones.

> 	 * c-family/c-attribs.c : introduce prolog_pad attribute and create
> 	   a handler for it.
>
> 	 * lto/lto-lang.c : Likewise.

 > 	 * testsuite/c-c++-common/attribute-prolog_pad-1.c : New test.

These three directories have separate ChangeLogs. The top-level 
directory name should be stripped and the entries added to the 
appropriate file.

> +      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))
> +	{
> +	  if (IDENTIFIER_LENGTH (get_attribute_name (*it)) !=
> +	      strlen("prolog_pad"))

Still several instances of bad formatting, in the entire block of code. 
Please refer to my previous email.

> +static tree
> +handle_prolog_pad_attribute (tree *, tree, tree, int,
> +			     bool *)
> +{
> +  /* Nothing to be done here.  */
> +  return NULL_TREE;
> +}

Should still add at least one-liner comments before these empty 
functions to say what interface they're implementing. Also, probably 
don't need to wrap the line containing the arguments.

> diff --git a/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> new file mode 100644
> index 0000000..2236aa8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attribute-prolog_pad-1.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fprolog-pad=3,1" } */

This test does not actually seem to verify that the padding has the 
expected size.

>   /* Do not use IPA optimizations for register allocation if profiler is active
> +    or prolog pads are inserted for run-time instrumentation
>      or port does not emit prologue and epilogue as RTL.  */
> -  if (profile_flag || !targetm.have_prologue () || !targetm.have_epilogue ())
> +  if (profile_flag || prolog_nop_pad_size
> +      || !targetm.have_prologue () || !targetm.have_epilogue ())
>      flag_ipa_ra = 0;

Was this explained? Why would ipa-ra be a problem?

> +  if (pad_size > pad_entry)
> +    targetm.asm_out.print_prolog_pad (asm_out_file, pad_size-pad_entry,
> +				      (pad_entry == 0));

Unnecessary parens around the last argument, and missing spaces around 
operators.


Bernd



More information about the Gcc-patches mailing list