Patchwork make it possible for the target to rename ".tm_clone_table"

login
register
mail settings
Submitter IainS
Date Nov. 18, 2011, 5:48 p.m.
Message ID <9B90055E-5374-4B02-8396-75502119DC89@sandoe-acoustics.co.uk>
Download mbox | patch
Permalink /patch/126457/
State New
Headers show

Comments

IainS - Nov. 18, 2011, 5:48 p.m.
At least Darwin needs a different name for the tm_clone_table section.

I'm wondering what a target without named sections does for this - is  
there some reason it needs to be in a separate section (from data)...

... perhaps what I'm missing here will help understand why the clone  
tests fail on *x68*-darwin :-)

comments/OK for trunk?
Iain

gcc:

	* defaults.h (TM_CLONE_TABLE_SECTION_NAME): Provide default
	tm clone section name.
	* varasm.c (dump_tm_clone_pairs): Use TM_CLONE_TABLE_SECTION_NAME.
	* config/darwin.h (TM_CLONE_TABLE_SECTION_NAME): Define.
Rainer Orth - Nov. 18, 2011, 5:56 p.m.
Iain Sandoe <developer@sandoe-acoustics.co.uk> writes:

> Index: gcc/defaults.h
> ===================================================================
> --- gcc/defaults.h	(revision 181476)
> +++ gcc/defaults.h	(working copy)
> @@ -392,6 +392,14 @@ see the files COPYING3 and COPYING.RUNTIME respect
>  #endif
>  #endif
>  
> +/* If we have named sections, provide a name for the transaction clone
> +   table section.  */
> +#if defined (TARGET_ASM_NAMED_SECTION)
> +#ifndef TM_CLONE_TABLE_SECTION_NAME
> +#define TM_CLONE_TABLE_SECTION_NAME ".tm_clone_table"
> +#endif
> +#endif
> +

This, together with the unconditional use in varasm.c, will lead to a
bootstrap failure on Tru64 UNIX, which lacks named sections completely.

	Rainer
IainS - Nov. 18, 2011, 6:06 p.m.
On 18 Nov 2011, at 17:56, Rainer Orth wrote:

> Iain Sandoe <developer@sandoe-acoustics.co.uk> writes:
>
>> Index: gcc/defaults.h
>> ===================================================================
>> --- gcc/defaults.h	(revision 181476)
>> +++ gcc/defaults.h	(working copy)
>> @@ -392,6 +392,14 @@ see the files COPYING3 and COPYING.RUNTIME  
>> respect
>> #endif
>> #endif
>>
>> +/* If we have named sections, provide a name for the transaction  
>> clone
>> +   table section.  */
>> +#if defined (TARGET_ASM_NAMED_SECTION)
>> +#ifndef TM_CLONE_TABLE_SECTION_NAME
>> +#define TM_CLONE_TABLE_SECTION_NAME ".tm_clone_table"
>> +#endif
>> +#endif
>> +
>
> This, together with the unconditional use in varasm.c, will lead to a
> bootstrap failure on Tru64 UNIX, which lacks named sections  
> completely.

right, it was worrying me what a target without named sections does -
... I can easily remove the #if defined (TARGET_ASM_NAMED_SECTION)
... what happens when the code runs then?

Iain
Rainer Orth - Nov. 18, 2011, 6:12 p.m.
Iain Sandoe <developer@sandoe-acoustics.co.uk> writes:

>> This, together with the unconditional use in varasm.c, will lead to a
>> bootstrap failure on Tru64 UNIX, which lacks named sections completely.
>
> right, it was worrying me what a target without named sections does -
> ... I can easily remove the #if defined (TARGET_ASM_NAMED_SECTION)
> ... what happens when the code runs then?

We ICE like this:

/vol/gcc/src/hg/trunk/local/libitm/testsuite/libitm.c/cancel.c:55:1: internal
compiler error: in default_no_named_section, at varasm.c:6293

I should probably file a PR, but there's still no proper bugzilla
component for trans-mem.

	Rainer
Joseph S. Myers - Nov. 18, 2011, 10:06 p.m.
On Fri, 18 Nov 2011, Iain Sandoe wrote:

> > > Index: gcc/defaults.h
> > > ===================================================================
> > > --- gcc/defaults.h	(revision 181476)
> > > +++ gcc/defaults.h	(working copy)
> > > @@ -392,6 +392,14 @@ see the files COPYING3 and COPYING.RUNTIME respect
> > > #endif
> > > #endif
> > > 
> > > +/* If we have named sections, provide a name for the transaction clone
> > > +   table section.  */
> > > +#if defined (TARGET_ASM_NAMED_SECTION)
> > > +#ifndef TM_CLONE_TABLE_SECTION_NAME
> > > +#define TM_CLONE_TABLE_SECTION_NAME ".tm_clone_table"
> > > +#endif
> > > +#endif
> > > +
> > 
> > This, together with the unconditional use in varasm.c, will lead to a
> > bootstrap failure on Tru64 UNIX, which lacks named sections completely.
> 
> right, it was worrying me what a target without named sections does -
> ... I can easily remove the #if defined (TARGET_ASM_NAMED_SECTION)
> ... what happens when the code runs then?

Conditions on definedness of TARGET_ASM_NAMED_SECTION are rather fragile - 
it's a target hook not properly a target macro, it shouldn't need to be 
defined in tm.h.  There are two such tests in defaults.h and two in 
target-def.h, but we should remove them rather than adding to them.  
TM_CLONE_TABLE_SECTION_NAME would better be a target hook, not a macro.

As I noted in <http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01642.html> 
(when cleaning up another such #ifdef conditional) there are actually very 
few targets without named sections.  And if we deprecate the a.out OpenBSD 
targets then there will be even fewer.

Patch

Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h	(revision 181476)
+++ gcc/defaults.h	(working copy)
@@ -392,6 +392,14 @@  see the files COPYING3 and COPYING.RUNTIME respect
 #endif
 #endif
 
+/* If we have named sections, provide a name for the transaction clone
+   table section.  */
+#if defined (TARGET_ASM_NAMED_SECTION)
+#ifndef TM_CLONE_TABLE_SECTION_NAME
+#define TM_CLONE_TABLE_SECTION_NAME ".tm_clone_table"
+#endif
+#endif
+
 /* If we have named section and we support weak symbols, then use the
    .jcr section for recording java classes which need to be registered
    at program start-up time.  */
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 181476)
+++ gcc/varasm.c	(working copy)
@@ -5961,7 +5961,9 @@  dump_tm_clone_pairs (VEC(tm_alias_pair,heap) *tm_a
 
       if (!switched)
 	{
-	  switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
+	  switch_to_section (get_named_section (NULL, 
+						TM_CLONE_TABLE_SECTION_NAME,
+						3));
 	  assemble_align (POINTER_SIZE);
 	  switched = true;
 	}
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 181476)
+++ gcc/config/darwin.h	(working copy)
@@ -692,6 +680,10 @@  extern GTY(()) section * darwin_sections[NUM_DARWI
 #define TARGET_ASM_UNIQUE_SECTION darwin_unique_section
 #undef  TARGET_ASM_FUNCTION_RODATA_SECTION
 #define TARGET_ASM_FUNCTION_RODATA_SECTION default_no_function_rodata_section
+
+#undef  TM_CLONE_TABLE_SECTION_NAME
+#define TM_CLONE_TABLE_SECTION_NAME "__DATA,__tm_clone_table"
+
 #undef  TARGET_ASM_RELOC_RW_MASK
 #define TARGET_ASM_RELOC_RW_MASK machopic_reloc_rw_mask