[PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

This patch adds to new backend hooks
ASM_OUTPUT_START_FUNCTION_HEADER and
ASM_OUTPUT_END_FUNCTION_FOOTER that may be defined to emit
assembly code at the very start or end of a function.  This
functionality is needed by the patch that ports the "target"
attribute and pragma to S/390.  (I'll post that patch shortly, and
in a different thread.)

Note that a function may need to switch to a different section in
the .s file (hot/cold section or whatever).  The additional code
is (arguably) emitted *before* switching the section.  That seems
to be more correct to me that emitting it after the switch, but
it's not a big deal to change that.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 312 bytes --]

gcc/ChangeLog

	* defaults.h (ASM_OUTPUT_START_FUNCTION_HEADER): New hook function.
	(ASM_OUTPUT_START_FUNCTION_FOOTER): Ditto.
	* varasm.c (assemble_start_function): Call new hook at start of
	function.
	(assemble_end_function): Call new hook at end of function.
	* doc/tm.texi.in: Document new hook functions.

[-- Attachment #3: 0001-Add-new-hooks-ASM_OUTPUT_START_FUNCTION_HEADER.patch --]
[-- Type: text/x-diff, Size: 4271 bytes --]

From 61c2546ba0aa0e9b2743f8d64427143aac5af2b0 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 29 Jul 2015 16:14:23 +0100
Subject: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

and ASM_OUTPUT_END_FUNCTION_FOOTER.  These are used by the implementation of
__attribute__ ((target(...))) on S390.
---
 gcc/defaults.h     | 10 ++++++++++
 gcc/doc/tm.texi    | 14 ++++++++++++++
 gcc/doc/tm.texi.in | 14 ++++++++++++++
 gcc/varasm.c       |  2 ++
 4 files changed, 40 insertions(+)

diff --git a/gcc/defaults.h b/gcc/defaults.h
index d4d3a56..aa9c849 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -135,6 +135,16 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define TARGET_DEFERRED_OUTPUT_DEFS(DECL,TARGET) false
 #endif
 
+/* Output additional assemler code at the very start of the function.  */
+#ifndef ASM_OUTPUT_START_FUNCTION_HEADER
+#define ASM_OUTPUT_START_FUNCTION_HEADER(FILE,DECL) do { } while (0)
+#endif
+
+/* Output additional assemler code at the very end of the function.  */
+#ifndef ASM_OUTPUT_END_FUNCTION_FOOTER
+#define ASM_OUTPUT_START_FUNCTION_FOOTER(FILE,DECL) do { } while (0)
+#endif
+
 /* This is how to output the definition of a user-level label named
    NAME, such as the label on variable NAME.  */
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index d548d96..7ba00c3 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7917,6 +7917,20 @@ If you define @code{TYPE_ASM_OP} and @code{TYPE_OPERAND_FMT}, a default
 definition of this macro is provided.
 @end defmac
 
+@defmac ASM_OUTPUT_START_FUNCTION_HEADER (@var{stream}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} additional assembly language code to be placed at the
+very start of the current function.  The default definition of this
+macro does nothing.
+@end defmac
+
+@defmac ASM_OUTPUT_END_FUNCTION_FOOTER (@var{stream}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} additional assembly language code at the very end of
+the current function.  The default definition of this macro does
+nothing.
+@end defmac
+
 @defmac ASM_DECLARE_FUNCTION_NAME (@var{stream}, @var{name}, @var{decl})
 A C statement (sans semicolon) to output to the stdio stream
 @var{stream} any text necessary for declaring the name @var{name} of a
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9bef4a5..2b2b7e4 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5546,6 +5546,20 @@ If you define @code{TYPE_ASM_OP} and @code{TYPE_OPERAND_FMT}, a default
 definition of this macro is provided.
 @end defmac
 
+@defmac ASM_OUTPUT_START_FUNCTION_HEADER (@var{stream}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} additional assembly language code to be placed at the
+very start of the current function.  The default definition of this
+macro does nothing.
+@end defmac
+
+@defmac ASM_OUTPUT_END_FUNCTION_FOOTER (@var{stream}, @var{decl})
+A C statement (sans semicolon) to output to the stdio stream
+@var{stream} additional assembly language code at the very end of
+the current function.  The default definition of this macro does
+nothing.
+@end defmac
+
 @defmac ASM_DECLARE_FUNCTION_NAME (@var{stream}, @var{name}, @var{decl})
 A C statement (sans semicolon) to output to the stdio stream
 @var{stream} any text necessary for declaring the name @var{name} of a
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 706e652..dcee8f0 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1701,6 +1701,7 @@ assemble_start_function (tree decl, const char *fnname)
   char tmp_label[100];
   bool hot_label_written = false;
 
+  ASM_OUTPUT_START_FUNCTION_HEADER (asm_out_file, current_function_decl);
   if (flag_reorder_blocks_and_partition)
     {
       ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LHOTB", const_labelno);
@@ -1864,6 +1865,7 @@ assemble_end_function (tree decl, const char *fnname ATTRIBUTE_UNUSED)
       ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.hot_section_end_label);
       switch_to_section (save_text_section);
     }
+  ASM_OUTPUT_END_FUNCTION_FOOTER (asm_out_file, current_function_decl);
 }
 \f
 /* Assemble code to leave SIZE bytes of zeros.  */
-- 
2.3.0

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Trevor Saunders]

On Mon, Sep 21, 2015 at 12:31:58PM +0100, Dominik Vogt wrote:
> This patch adds to new backend hooks
> ASM_OUTPUT_START_FUNCTION_HEADER and
> ASM_OUTPUT_END_FUNCTION_FOOTER that may be defined to emit
> assembly code at the very start or end of a function.  This
> functionality is needed by the patch that ports the "target"
> attribute and pragma to S/390.  (I'll post that patch shortly, and
> in a different thread.)

obligatory question, why are we adding new target macros instead of
using hooks?  Especially in a place like this where the caller is very
unlikely to be at all hot.

thanks!

Trev

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Jeff Law]

On 09/21/2015 05:31 AM, Dominik Vogt wrote:
> This patch adds to new backend hooks
> ASM_OUTPUT_START_FUNCTION_HEADER and
> ASM_OUTPUT_END_FUNCTION_FOOTER that may be defined to emit
> assembly code at the very start or end of a function.  This
> functionality is needed by the patch that ports the "target"
> attribute and pragma to S/390.  (I'll post that patch shortly, and
> in a different thread.)
>
> Note that a function may need to switch to a different section in
> the .s file (hot/cold section or whatever).  The additional code
> is (arguably) emitted *before* switching the section.  That seems
> to be more correct to me that emitting it after the switch, but
> it's not a big deal to change that.
Is there some good reason these aren't hooks?

Jeff

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

On Tue, Sep 22, 2015 at 01:56:15PM -0600, Jeff Law wrote: > Is
there some good reason these aren't hooks?

No, that was just inobservance.  New version attached.  Would it be
preferrable to initialize the hooks with a NULL pointer and test
the pointer before calling them?  (That way the changes to
hooks.[ch] could be dropped.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 337 bytes --]

gcc/ChangeLog

	* target.def: Add function_start and function_end hooks.
	* hooks.c (hook_void_FILEptr_tree): New function.
	* hooks.h: Ditto.
	* varasm.c (assemble_start_function): Call hook at start of function.
	(assemble_end_function): Call hook at end of function.
	* doc/tm.texi.in: Document new hooks.
	* doc/tm.texi: Regenerate.

[-- Attachment #3: 0001-Add-new-hooks-asm_out.function_start-and-asm_out.fun.patch --]
[-- Type: text/x-diff, Size: 4653 bytes --]

From 791b0dc5ba32ace51fb8214cdb0cf769b91a024c Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Wed, 29 Jul 2015 16:14:23 +0100
Subject: [PATCH] Add new hooks asm_out.function_start and
 asm_out.function_end.

They are used by the implementation of __attribute__ ((target(...))) on S390.
---
 gcc/doc/tm.texi    | 10 ++++++++++
 gcc/doc/tm.texi.in |  4 ++++
 gcc/hooks.c        |  7 +++++++
 gcc/hooks.h        |  1 +
 gcc/target.def     | 16 ++++++++++++++++
 gcc/varasm.c       |  2 ++
 6 files changed, 40 insertions(+)

diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index d548d96..62d83db 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -7348,6 +7348,16 @@ Output to @code{asm_out_file} any text which the assembler expects
 to find at the end of a file.  The default is to output nothing.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_ASM_FUNCTION_START (FILE *@var{}, @var{tree})
+Output to @code{asm_out_file} any text which is necessary at the start of
+a function.  The default is to output nothing.
+@end deftypefn
+
+@deftypefn {Target Hook} void TARGET_ASM_FUNCTION_END (FILE *@var{}, @var{tree})
+Output to @code{asm_out_file} any text which is necessary at the end of a
+function.  The default is to output nothing.
+@end deftypefn
+
 @deftypefun void file_end_indicate_exec_stack ()
 Some systems use a common convention, the @samp{.note.GNU-stack}
 special section, to indicate whether or not an object file relies on
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 9bef4a5..b1c4b96 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -5122,6 +5122,10 @@ This describes the overall framework of an assembly file.
 
 @hook TARGET_ASM_FILE_END
 
+@hook TARGET_ASM_FUNCTION_START
+
+@hook TARGET_ASM_FUNCTION_END
+
 @deftypefun void file_end_indicate_exec_stack ()
 Some systems use a common convention, the @samp{.note.GNU-stack}
 special section, to indicate whether or not an object file relies on
diff --git a/gcc/hooks.c b/gcc/hooks.c
index 0fb9add..3440e06 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -146,6 +146,13 @@ hook_void_FILEptr_constcharptr_const_tree (FILE *, const char *, const_tree)
 {
 }
 
+/* Generic hook that takes (FILE *, tree) and does
+   nothing.  */
+void
+hook_void_FILEptr_tree (FILE *, tree)
+{
+}
+
 /* Generic hook that takes (FILE *, rtx) and returns false.  */
 bool
 hook_bool_FILEptr_rtx_false (FILE *a ATTRIBUTE_UNUSED,
diff --git a/gcc/hooks.h b/gcc/hooks.h
index c3d4bd3..bbd26cb 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -70,6 +70,7 @@ extern void hook_void_rtx_insn_int (rtx_insn *, int);
 extern void hook_void_FILEptr_constcharptr (FILE *, const char *);
 extern void hook_void_FILEptr_constcharptr_const_tree (FILE *, const char *,
 						       const_tree);
+extern void hook_void_FILEptr_tree (FILE *, tree);
 extern bool hook_bool_FILEptr_rtx_false (FILE *, rtx);
 extern void hook_void_rtx_tree (rtx, tree);
 extern void hook_void_tree (tree);
diff --git a/gcc/target.def b/gcc/target.def
index aa5a1f1..4a18be5 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -672,6 +672,22 @@ to find at the end of a file.  The default is to output nothing.",
  void, (void),
  hook_void_void)
 
+/* Output additional text at the start of a function.  */
+DEFHOOK
+(function_start,
+ "Output to @code{asm_out_file} any text which is necessary at the start of\n\
+a function.  The default is to output nothing.",
+ void, (FILE *, tree),
+ hook_void_FILEptr_tree)
+
+/* Output additional text at the end of a function.  */
+DEFHOOK
+(function_end,
+ "Output to @code{asm_out_file} any text which is necessary at the end of a\n\
+function.  The default is to output nothing.",
+ void, (FILE *, tree),
+ hook_void_FILEptr_tree)
+
 /* Output any boilerplate text needed at the beginning of an
    LTO output stream.  */
 DEFHOOK
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 706e652..1b6f7b7 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1701,6 +1701,7 @@ assemble_start_function (tree decl, const char *fnname)
   char tmp_label[100];
   bool hot_label_written = false;
 
+  targetm.asm_out.function_start (asm_out_file, current_function_decl);
   if (flag_reorder_blocks_and_partition)
     {
       ASM_GENERATE_INTERNAL_LABEL (tmp_label, "LHOTB", const_labelno);
@@ -1864,6 +1865,7 @@ assemble_end_function (tree decl, const char *fnname ATTRIBUTE_UNUSED)
       ASM_OUTPUT_LABEL (asm_out_file, crtl->subsections.hot_section_end_label);
       switch_to_section (save_text_section);
     }
+  targetm.asm_out.function_end (asm_out_file, current_function_decl);
 }
 \f
 /* Assemble code to leave SIZE bytes of zeros.  */
-- 
2.3.0

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Bernd Schmidt]

On 09/23/2015 04:48 PM, Dominik Vogt wrote:
> On Tue, Sep 22, 2015 at 01:56:15PM -0600, Jeff Law wrote: > Is
> there some good reason these aren't hooks?
>
> No, that was just inobservance.  New version attached.  Would it be
> preferrable to initialize the hooks with a NULL pointer and test
> the pointer before calling them?  (That way the changes to
> hooks.[ch] could be dropped.)

There are already several hooks/macros in use for this kind of thing, 
have you checked that they are not usable for your purpose? There's 
ASM_DECLARE_FUNCTION_NAME, which is used by nvptx for example, and 
there's also ASM_OUTPUT_FUNCTION_PREFIX, which is apparently used by 
nothing in the current tree. For the end you could use 
ASM_DECLARE_FUNCTION_SIZE.

FWIW I prefer the initialization with functions rather than NULL.


Bernd

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

On Thu, Sep 24, 2015 at 12:41:24PM +0200, Bernd Schmidt wrote:
> On 09/23/2015 04:48 PM, Dominik Vogt wrote:
> >On Tue, Sep 22, 2015 at 01:56:15PM -0600, Jeff Law wrote: > Is
> >there some good reason these aren't hooks?
> >
> >No, that was just inobservance.  New version attached.  Would it be
> >preferrable to initialize the hooks with a NULL pointer and test
> >the pointer before calling them?  (That way the changes to
> >hooks.[ch] could be dropped.)
> 
> There are already several hooks/macros in use for this kind of
> thing, have you checked that they are not usable for your purpose?
> There's ASM_DECLARE_FUNCTION_NAME, which is used by nvptx for
> example, and there's also ASM_OUTPUT_FUNCTION_PREFIX, which is
> apparently used by nothing in the current tree. For the end you
> could use ASM_DECLARE_FUNCTION_SIZE.

Hm, ASM_DECLARE_FUNCTION_NAME, ASM_DECLARE_FUNCTION_LABEL and
ASM_OUTPUT_FUNCTION_PREFIX are all called too late in the code to
be useful.  The new hook (or any replacement for it) must be
emitted:

 * before any alignmanet of the function is done,
 * before the constant pool is generated (however, s390 places the
   constant pool after the function anyway).

ASM_DECLARE_FUNCTION_SIZE on the ohter hand is called too early;
the end hook must be called after generating the constant pool.
For example, activating or deactivating the vector extension on a
z13 affects alignment of vector type constants.  So,
unfortunately, I see no way to make use of the existing hooks.
You're right that there are plenty already.

Hm, I wonder whether wrapping all these section switches in
assemble_start/end_function in ".machine" pseudoops (that's what
we need the hooks for; similar to .arch for ix86) has any real
effect.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Bernd Schmidt]

On 09/24/2015 03:48 PM, Dominik Vogt wrote:
> Hm, I wonder whether wrapping all these section switches in
> assemble_start/end_function in ".machine" pseudoops (that's what
> we need the hooks for; similar to .arch for ix86) has any real
> effect.

I don't think I follow what you're trying to say here?


Bernd

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

On Fri, Sep 25, 2015 at 03:33:56PM +0200, Bernd Schmidt wrote:
> On 09/24/2015 03:48 PM, Dominik Vogt wrote:
> >Hm, I wonder whether wrapping all these section switches in
> >assemble_start/end_function in ".machine" pseudoops (that's what
> >we need the hooks for; similar to .arch for ix86) has any real
> >effect.
> 
> I don't think I follow what you're trying to say here?

I mean, it's more or less random whether switching to and from the
function's section ends up inside the new .machine and
.machinemode directives (if the section needs to be switched for
this function) or outside (if the assembler code had already
switched to the correct section earlier).  I assume that .machine
and .machinemode have no effect on the section switching, but I'm
not completely sure (alignment?).

(@Andreas + Uli: Do you know of any effect this would have on
s390?)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Bernd Schmidt]

On 09/28/2015 11:44 AM, Dominik Vogt wrote:
> On Fri, Sep 25, 2015 at 03:33:56PM +0200, Bernd Schmidt wrote:
>> On 09/24/2015 03:48 PM, Dominik Vogt wrote:
>>> Hm, I wonder whether wrapping all these section switches in
>>> assemble_start/end_function in ".machine" pseudoops (that's what
>>> we need the hooks for; similar to .arch for ix86) has any real
>>> effect.
>>
>> I don't think I follow what you're trying to say here?
>
> I mean, it's more or less random whether switching to and from the
> function's section ends up inside the new .machine and
> .machinemode directives (if the section needs to be switched for
> this function) or outside (if the assembler code had already
> switched to the correct section earlier).  I assume that .machine
> and .machinemode have no effect on the section switching, but I'm
> not completely sure (alignment?).
>
> (@Andreas + Uli: Do you know of any effect this would have on
> s390?)

Still not really following since I don't know anything about s390 and 
its directives. In case you're trying to figure out whether it's 
possible to use the existing macros, please continue doing so. If you 
reach the conclusion that you really do need the new hooks, your patch 
is ok. However, you probably should add a sentence or two to the 
documentation to specify ordering wrt other parts of the header of a 
function.


Bernd

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

On Mon, Sep 28, 2015 at 01:54:17PM +0200, Bernd Schmidt wrote:
> On 09/28/2015 11:44 AM, Dominik Vogt wrote:
> >I mean, it's more or less random whether switching to and from the
> >function's section ends up inside the new .machine and
> >.machinemode directives (if the section needs to be switched for
> >this function) or outside (if the assembler code had already
> >switched to the correct section earlier).  I assume that .machine
> >and .machinemode have no effect on the section switching, but I'm
> >not completely sure (alignment?).
> 
> Still not really following since I don't know anything about s390
> and its directives.

.machine switches the Cpu (like .arch on x86) and .machinemode
switches between Esa and Zarch modes.


> In case you're trying to figure out whether it's possible to use
> the existing macros, please continue doing so.

I don't see any way to use existing hooks for that (because the
constant pool needs to be wrapped inside the hooks and there are
none that do that at the moment).

> However, you probably should add a sentence or two to
> the documentation to specify ordering wrt other parts of the header
> of a function.

Any suggestions where that information should be placed in the
documentation?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Bernd Schmidt]

On 10/01/2015 08:30 AM, Dominik Vogt wrote:

>> However, you probably should add a sentence or two to
>> the documentation to specify ordering wrt other parts of the header
>> of a function.
>
> Any suggestions where that information should be placed in the
> documentation?

Just in the hook definition.


Bernd

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

On Mon, Sep 21, 2015 at 12:31:58PM +0100, Dominik Vogt wrote:
> This patch adds to new backend hooks
> ASM_OUTPUT_START_FUNCTION_HEADER and
> ASM_OUTPUT_END_FUNCTION_FOOTER that may be defined to emit
> assembly code at the very start or end of a function.

We no longer need this patch.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Add new hooks ASM_OUTPUT_START_FUNCTION_HEADER ...

[Dominik Vogt]

On Mon, Sep 21, 2015 at 12:31:58PM +0100, Dominik Vogt wrote:
> This patch adds to new backend hooks
> ASM_OUTPUT_START_FUNCTION_HEADER and
> ASM_OUTPUT_END_FUNCTION_FOOTER that may be defined to emit
> assembly code at the very start or end of a function.

We no longer need this patch.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany