diff mbox

[c] Fix target/57848: internal compiler error on builtin and '#pragma GCC target()' option

Message ID CAEwic4Y5R8+PuwL+m20B1WMfe4J8SJWUzSOsJw5XQkV4OGX-vg@mail.gmail.com
State New
Headers show

Commit Message

Kai Tietz Sept. 13, 2013, 7:51 a.m. UTC 
Hello,

this patch fixes a wrong assumption in c_builtin_function_ext_scope.
The check for never being called on preexisting symbols (with
meaning), isn't correct as the turning on of builtins via pragmas
wasn't considered.

The following sample demonstrate this issue pretty well (it is reduced
testcase of failure occuring in i386 intrinsics):

extern unsigned int __builtin_ia32_crc32si (unsigned int, unsigned int);
#pragma GCC target("sse4.2")

To be compiled for 32-bit mode without enabled sse.


ChangeLog

2013-09-13  Kai Tietz  <ktietz@redhat.com>

    PR target/57484
    * c/c-decl.c (c_builtin_function_ext_scope): Remove
    wrong assumption that it is never called on prexisting
    symbol.

Tested for i686-w64-mingw32, x86_64-w64-mingw32,
x86_64-unknown-linux-gnu (multilib).  Ok for apply?

Regards,
Kai

Comments

Mikael Pettersson Sept. 13, 2013, 12:15 p.m. UTC | #1 
Kai Tietz writes:
 > Hello,
 > 
 > this patch fixes a wrong assumption in c_builtin_function_ext_scope.
 > The check for never being called on preexisting symbols (with
 > meaning), isn't correct as the turning on of builtins via pragmas
 > wasn't considered.
 > 
 > The following sample demonstrate this issue pretty well (it is reduced
 > testcase of failure occuring in i386 intrinsics):
 > 
 > extern unsigned int __builtin_ia32_crc32si (unsigned int, unsigned int);
 > #pragma GCC target("sse4.2")
 > 
 > To be compiled for 32-bit mode without enabled sse.
 > 
 > 
 > ChangeLog
 > 
 > 2013-09-13  Kai Tietz  <ktietz@redhat.com>
 > 
 >     PR target/57484

Typo, please s/57484/57848/ here.

 >     * c/c-decl.c (c_builtin_function_ext_scope): Remove
 >     wrong assumption that it is never called on prexisting
 >     symbol.
Joseph Myers Sept. 13, 2013, 4:40 p.m. UTC | #2 
On Fri, 13 Sep 2013, Kai Tietz wrote:

> ChangeLog
> 
> 2013-09-13  Kai Tietz  <ktietz@redhat.com>
> 
>     PR target/57484
>     * c/c-decl.c (c_builtin_function_ext_scope): Remove
>     wrong assumption that it is never called on prexisting
>     symbol.

c/ has its own ChangeLog so the entry should go there without the initial 
c/ in the file name in the ChangeLog entry.

The patch is OK with the addition of the testcase you gave to 
gcc.target/i386/.
diff mbox

Patch

Index: c-decl.c
===================================================================
--- c-decl.c    (Revision 202554)
+++ c-decl.c    (Arbeitskopie)
@@ -3629,9 +3629,6 @@  c_builtin_function_ext_scope (tree decl)
   const char *name = IDENTIFIER_POINTER (id);
   C_DECL_BUILTIN_PROTOTYPE (decl) = prototype_p (type);

-  /* Should never be called on a symbol with a preexisting meaning.  */
-  gcc_assert (!I_SYMBOL_BINDING (id));
-
   bind (id, decl, external_scope, /*invisible=*/false, /*nested=*/false,
     UNKNOWN_LOCATION);