diff mbox

Fix librayr name of __builtin_allocal_with_align

Message ID 20150427083126.GB46857@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka April 27, 2015, 8:31 a.m. UTC
Hi,
build_common_builtin_nodes declares both __builtin_alloca and
__builtin_alloca_with_align to have library name "alloca". This actually
triggers warning in an updated ODR violation detector on "alloca" being
declared twice.

__builtin_alloca_with_align IMO do not have library equivalent and I think this
is a pasto (__builtin_alloca_with_align is not documented in extend.texi). It
is not clear to me if there was some intention behind this oddity though.

I have bootstrapped/regtested x86_64 with the following. OK?

Honza

	* tree.c (build_common_builtin_nodes): Do not build
	__builtin_alloca_with_align as equivalent of library alloca.

Comments

Richard Biener April 27, 2015, 8:47 a.m. UTC | #1
On Mon, 27 Apr 2015, Jan Hubicka wrote:

> Hi,
> build_common_builtin_nodes declares both __builtin_alloca and
> __builtin_alloca_with_align to have library name "alloca". This actually
> triggers warning in an updated ODR violation detector on "alloca" being
> declared twice.
> 
> __builtin_alloca_with_align IMO do not have library equivalent and I think this
> is a pasto (__builtin_alloca_with_align is not documented in extend.texi). It
> is not clear to me if there was some intention behind this oddity though.
> 
> I have bootstrapped/regtested x86_64 with the following. OK?

Yes.

Thanks,
Richard.

> Honza
> 
> 	* tree.c (build_common_builtin_nodes): Do not build
> 	__builtin_alloca_with_align as equivalent of library alloca.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 222391)
> +++ tree.c	(working copy)
> @@ -10088,7 +10098,8 @@ build_common_builtin_nodes (void)
>    ftype = build_function_type_list (ptr_type_node, size_type_node,
>  				    size_type_node, NULL_TREE);
>    local_define_builtin ("__builtin_alloca_with_align", ftype,
> -			BUILT_IN_ALLOCA_WITH_ALIGN, "alloca",
> +			BUILT_IN_ALLOCA_WITH_ALIGN,
> +			"__builtin_alloca_with_align",
>  			ECF_MALLOC | ECF_NOTHROW | ECF_LEAF);
>  
>    /* If we're checking the stack, `alloca' can throw.  */
>
Jeff Law April 28, 2015, 11:01 p.m. UTC | #2
On 04/27/2015 02:31 AM, Jan Hubicka wrote:
> Hi,
> build_common_builtin_nodes declares both __builtin_alloca and
> __builtin_alloca_with_align to have library name "alloca". This actually
> triggers warning in an updated ODR violation detector on "alloca" being
> declared twice.
>
> __builtin_alloca_with_align IMO do not have library equivalent and I think this
> is a pasto (__builtin_alloca_with_align is not documented in extend.texi). It
> is not clear to me if there was some intention behind this oddity though.
>
> I have bootstrapped/regtested x86_64 with the following. OK?
>
> Honza
>
> 	* tree.c (build_common_builtin_nodes): Do not build
> 	__builtin_alloca_with_align as equivalent of library alloca.
Yea, it's probably a pasto/thinko.  OK for the trunk.

jeff
Jan Hubicka April 29, 2015, 2:22 a.m. UTC | #3
> On 04/27/2015 02:31 AM, Jan Hubicka wrote:
> >Hi,
> >build_common_builtin_nodes declares both __builtin_alloca and
> >__builtin_alloca_with_align to have library name "alloca". This actually
> >triggers warning in an updated ODR violation detector on "alloca" being
> >declared twice.
> >
> >__builtin_alloca_with_align IMO do not have library equivalent and I think this
> >is a pasto (__builtin_alloca_with_align is not documented in extend.texi). It
> >is not clear to me if there was some intention behind this oddity though.
> >
> >I have bootstrapped/regtested x86_64 with the following. OK?
> >
> >Honza
> >
> >	* tree.c (build_common_builtin_nodes): Do not build
> >	__builtin_alloca_with_align as equivalent of library alloca.
> Yea, it's probably a pasto/thinko.  OK for the trunk.

Thank you!
I also looked into extend.texi and both __builtin_alloca and __builtin_alloca_with_align
is missing (along with many other builtins) which is probably quite bad omission
(__builtin_alloca_with_align is used at some places, i.e. in Firefox)

Sandra, since you d oreat job on updating the docs recently, perhaps this could
get at your radar?

Honza
> 
> jeff
Sandra Loosemore April 29, 2015, 3:22 p.m. UTC | #4
On 04/28/2015 08:22 PM, Jan Hubicka wrote:

> I also looked into extend.texi and both __builtin_alloca and __builtin_alloca_with_align
> is missing (along with many other builtins) which is probably quite bad omission
> (__builtin_alloca_with_align is used at some places, i.e. in Firefox)
>
> Sandra, since you d oreat job on updating the docs recently, perhaps this could
> get at your radar?

Hmmmm.  I have quite a lot of other stuff in my queue at the moment 
(including a bunch of nios2 patches, not just documentation work) and I 
might forget about this before I have time to work on it.  I'd encourage 
people who notice omissions in the manual to take a stab at fixing it 
themselves.  Alternatively, you could file PRs for documentation bugs, 
or maybe we should have a documentation improvements section on the wiki 
so that people besides me who have time/interest could find chunks of 
the backlog to chip away at.

-Sandra
diff mbox

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 222391)
+++ tree.c	(working copy)
@@ -10088,7 +10098,8 @@  build_common_builtin_nodes (void)
   ftype = build_function_type_list (ptr_type_node, size_type_node,
 				    size_type_node, NULL_TREE);
   local_define_builtin ("__builtin_alloca_with_align", ftype,
-			BUILT_IN_ALLOCA_WITH_ALIGN, "alloca",
+			BUILT_IN_ALLOCA_WITH_ALIGN,
+			"__builtin_alloca_with_align",
 			ECF_MALLOC | ECF_NOTHROW | ECF_LEAF);
 
   /* If we're checking the stack, `alloca' can throw.  */