Patchwork fix regression after linux.h / gnu-user.h split

login
register
mail settings
Submitter Robert Millan
Date Jan. 18, 2011, 8:01 p.m.
Message ID <AANLkTimkJ2UukvEJG7rUSQLiSmz3YZN8nYbNE=HoJMUy@mail.gmail.com>
Download mbox | patch
Permalink /patch/79347/
State New
Headers show

Comments

Robert Millan - Jan. 18, 2011, 8:01 p.m.
2011/1/12 Richard Guenther <richard.guenther@gmail.com>:
> On Wed, Jan 12, 2011 at 1:44 PM, Joseph S. Myers
> <joseph@codesourcery.com> wrote:
>> Ping^2.  This patch
>> <http://gcc.gnu.org/ml/gcc-patches/2010-12/msg02055.html> (with typo fix
>> as noted in <http://gcc.gnu.org/ml/gcc-patches/2010-12/msg02056.html>) is
>> pending review.
>
> Ok.

Sorry for noticing too late, but this patch introduced a small regression:
TARGET_C99_FUNCTIONS and TARGET_HAS_SINCOS, which
are always wanted on Glibc, are not defined anymore for GNU
systems on kernels other than Linux.

Patch with ChangeLog entry attached.
Robert Millan - Jan. 18, 2011, 9:34 p.m.
2011/1/18 Joseph S. Myers <joseph@codesourcery.com>:
> The key principle of this patch is that *targets use gnu-user.h if and
> only if they use linux.h*.

Sorry for the confusion, I was under the assumption that after your
patch linux.h was only used by Linux-based targets.  Is this meant
for a newer patch?

> Defining TARGET_C99_FUNCTIONS and TARGET_HAS_SINCOS in the correct
> target-dependent way is explicitly meant to be a subsequent, more risky,
> patch (and this would not be the same approach as in your patch, but a
> more complicated change involving config.gcc as well).

Ok.  When you have more details, could you keep me on CC?

Thanks
Joseph S. Myers - Jan. 18, 2011, 9:44 p.m.
On Tue, 18 Jan 2011, Robert Millan wrote:

> 2011/1/18 Joseph S. Myers <joseph@codesourcery.com>:
> > The key principle of this patch is that *targets use gnu-user.h if and
> > only if they use linux.h*.
> 
> Sorry for the confusion, I was under the assumption that after your
> patch linux.h was only used by Linux-based targets.  Is this meant
> for a newer patch?

Yes.  Such a patch will also define TARGET_C99_FUNCTIONS and 
TARGET_HAS_SINCOS to 1 in headers such as gnu.h and remove some 
definitions in config.gcc that are no longer needed after that change.
Robert Millan - Jan. 18, 2011, 9:55 p.m.
2011/1/18 Joseph S. Myers <joseph@codesourcery.com>:
> Yes.  Such a patch will also define TARGET_C99_FUNCTIONS and
> TARGET_HAS_SINCOS to 1 in headers such as gnu.h and remove some
> definitions in config.gcc that are no longer needed after that change.

Please consider defining them in generic gnu-user.h and
overriden in linux.h.  This way all the other systems share
the same definition, which makes it harder for bitrot to
happen when either:

  - New Glibc-specific definitions are added.
  - TARGET_C99_FUNCTIONS or TARGET_HAS_SINCOS
    is modified.

Contrary, if each of these 4 systems has its own definition
of TARGET_C99_FUNCTIONS and TARGET_HAS_SINCOS
just to assert they're using Glibc, it's much more likely that
they get out of sync in the future.

Patch

2011-01-18  Robert Millan  <rmh@gnu.org>

	* config/gnu-user.h (TARGET_C99_FUNCTIONS, TARGET_HAS_SINCOS): Define
	to 1 as default for most GNU systems.
	* config/linux.h (TARGET_C99_FUNCTIONS, TARGET_HAS_SINCOS): Undefine
	before defining, so that new value takes precedence.

Index: gcc/config/linux.h
===================================================================
--- gcc/config/linux.h	(revision 168952)
+++ gcc/config/linux.h	(working copy)
@@ -93,7 +93,9 @@ 
 
 /* Determine whether the entire c99 runtime
    is present in the runtime library.  */
+#undef TARGET_C99_FUNCTIONS
 #define TARGET_C99_FUNCTIONS (OPTION_GLIBC)
 
 /* Whether we have sincos that follows the GNU extension.  */
+#undef TARGET_HAS_SINCOS
 #define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC)
Index: gcc/config/gnu-user.h
===================================================================
--- gcc/config/gnu-user.h	(revision 168952)
+++ gcc/config/gnu-user.h	(working copy)
@@ -95,3 +95,10 @@ 
 #endif
 
 #define TARGET_POSIX_IO
+
+/* Whether the entire c99 runtime is present in the runtime
+   library.  */
+#define TARGET_C99_FUNCTIONS 1
+
+/* Whether we have sincos that follows the GNU extension.  */   
+#define TARGET_HAS_SINCOS 1