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

login
register
mail settings
Submitter Kai Tietz
Date Sept. 13, 2013, 7:51 a.m.
Message ID <CAEwic4Y5R8+PuwL+m20B1WMfe4J8SJWUzSOsJw5XQkV4OGX-vg@mail.gmail.com>
Download mbox | patch
Permalink /patch/274672/
State New
Headers show

Comments

Kai Tietz - Sept. 13, 2013, 7:51 a.m.
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
Mikael Pettersson - Sept. 13, 2013, 12:15 p.m.
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 S. Myers - Sept. 13, 2013, 4:40 p.m.
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/.

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);