This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v3] add -fprolog-pad=N,M option
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: Torsten Duwe <duwe at suse dot de>, Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, nd at arm dot com, Li Bin <huawei dot libin at huawei dot com>, Jiri Kosina <jkosina at suse dot cz>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Takahiro Akashi <takahiro dot akashi at linaro dot org>, Andrew Wafaa <Andrew dot Wafaa at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Mon, 19 Dec 2016 16:04:28 +0100
- Subject: Re: [PATCH v3] add -fprolog-pad=N,M option
- Authentication-results: sourceware.org; auth=none
- References: <20161216141444.GA29480@suse.de>
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