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

[Bernd Schmidt]

I'll consider myself agnostic as to whether this is a feature we want or 
need, so I'll just comment on some style questions. There's a fair 
amount of coding style violations, I'll point some of them out but 
please read the documents we have linked on this page:
   https://gcc.gnu.org/contribute.html

On 12/16/2016 03:14 PM, Torsten Duwe wrote:
> Signed-off-by: Torsten Duwe <duwe@suse.de>

This is meaningless for the GCC project. We require a copyright 
assignment; I assume SuSE has a blanket one that covers you. You should 
also write a ChangeLog entry.

> diff --git a/gcc/attribs.c b/gcc/attribs.c
> index e66349a..6ff81a8 100644
> --- a/gcc/attribs.c
> +++ b/gcc/attribs.c
> @@ -365,6 +365,28 @@ decl_attributes (tree *node, tree attributes, int flags)
>    if (!attributes_initialized)
>      init_attributes ();
>
> +  /* If we're building NOP pads because of a command line arg, note the size
> +     for LTO builds, unless the attribute has already been overridden. */

Two spaces at the end of a sentence, including at the end of a comment.

> +  if (TREE_CODE (*node) == FUNCTION_DECL &&
> +      prolog_nop_pad_size > 0)

Operators go on the next line when wrapping.

> +					     ),

Don't put closing parens on their own line.

> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index db293fe..7f3e558 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -2322,6 +2322,34 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, tree oldtype)
>        TREE_ASM_WRITTEN (olddecl) = 0;
>      }
>
> +  /* Prolog pad size may be set wrongly by a forward declaration;
> +     fix it up by pulling the final value in front.
> +  */

The "*/" should go on the previous line.

> +      for (it = &DECL_ATTRIBUTES (newdecl); *it; it = &TREE_CHAIN(*it))

Space before opening parentheses (many occurrences).
>
> +void
> +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
> +			  bool record_p)

Every function needs a comment describing its purpose and that of its 
arguments. Look around for examples. There are cases in this patch of 
hook implementations which ignore all their args, there I believe we can 
relax this rule a little (but a comment saying which hook is implemented 
would still be good).

>
> +extern void default_print_prolog_pad (FILE *, unsigned HOST_WIDE_INT , bool);

Careful with stray whitespace.
> +      if (tree_fits_uhwi_p (prolog_pad_value1) )

Here too.
> +	{
> +	  pad_size = tree_to_uhwi (prolog_pad_value1);
> +	}

Lose { } braces around single statements. Several cases.

Am I missing it or is there no actual implementation of the target hook 
anywhere?


Bernd