This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]