[PATCH] Fix gcc.dg/torture/pr26565.c at -O0 on SPARC/SH

Roger Sayle roger@eyesopen.com
Sat Apr 29 15:32:00 GMT 2006


Hi Eric,

On Mon, 27 Mar 2006, Eric Botcazou wrote:
http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01569.html
> 2006-03-27  Eric Botcazou  <ebotcazou@libertysurf.fr>
>
> 	* varasm.c (process_pending_assemble_externals): Set to 0
> 	unconditionally.
> 	(assemble_external): Process the argument unconditionally.
> 	Invoke assemble_external_real if ASM_OUTPUT_EXTERNAL, otherwise
> 	mimic its side-effects.

I don't think this is the correct fix.  I've just (re)discoverd the that
the person who added the DECL_ASSEMBLER_NAME_SET check to expand_builtin
was me, as revision 70195.  The intention was that builtin functions
taht have real library equivalents have/had their DECL_ASSEMBLER_NAME
set at the point the builtin function was created.  This could then be
use in the middle-end to distinguish __builtin_memset from
__builtin_constant_p, to avoid calling a mythical "constant_p".

The code responsible for setting a builtin's DECL_ASSEMBLER_NAME is
in c-decl.c's builtin_function which contains the lines:

  if (library_name)
    SET_DECL_ASSEMBLER_NAME (decl, get_identifier (library_name));

which appears to be doing exactly what we want.  However digging a
little deeper it appears that the true bug may lie in c-common.c's
def_builtin_1 which contains the section:

  libname = name + strlen ("__builtin_");
  decl = lang_hooks.builtin_function (name, fntype, fncode, fnclass,
                                      (fallback_p ? libname : NULL),
                                      fnattrs);
  if (both_p
      && !flag_no_builtin && !builtin_function_disabled_p (libname)
      && !(nonansi_p && flag_no_nonansi_builtin))
    lang_hooks.builtin_function (libname, libtype, fncode, fnclass,
                                 NULL, fnattrs);

Notice that in the first call to the builtin_function langhook, the
second to last argument specifies a libname when fallback_p is true,
but that in the second call we always specify NULL.  I believe that
this second call needs to use the same "(fallback_p ? libname : NULL)"
idiom as the first.

Certainly that was the intention of the DECL_ASSEMBLER_NAME_SET_P
test, and the mechanism used to convey the fallback_p flag from
here at definition time, to RTL expansion time.  That some targets
could also fill this field in for themselves, an accidental
side-effect.

Investigating further, the problem was introduced by revision 102911
of c-common.c where the transition to using the builtin_function
langhook accidentally changed these semantics.


Could you try/bootstrap/regtest the above fix and see if that cures
the failure of pr26565.c at -O0 on SPARC/SH?  Perhaps there's a
conflict and a valid reason why this field isn't currently being
initialied here?


Sorry for the delay.  On my first reading of your varasm.c change
I thought this was a "varasm" issue, and hoped for someone else to
comment.

Roger
--



More information about the Gcc-patches mailing list