public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] shut up warnings in bits/string2.h
@ 1999-12-07 13:02 Jakub Jelinek
  1999-12-07 18:41 ` Richard Henderson
  1999-12-07 23:41 ` Ulrich Drepper
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 1999-12-07 13:02 UTC (permalink / raw)
  To: drepper; +Cc: libc-hacker

Hi!

This patch shuts up gcc 2.96 warnings about long switch expressions not
converted to int.
BTW: I've seen pretty bad code as result of the string2.h inlines on sparc,
while -D__NO_STRING_INLINES gave me much better code as the compiler did the
job. I'll investigate why that happens and for which functions and submit a
sparc string.h which will disable those inlines from string2.h which result
in bad code. How does it work on other architectures?

1999-12-07  Jakub Jelinek  <jakub@redhat.com>

	* string/bits/string2.h (__memset_gc, __mempcpy_small, __strcpy_small,
	__stpcpy_small): Cast switch expressions to int to shut up compiler
	warnings.

--- string/bits/string2.h.jj12	Thu Oct 14 21:29:59 1999
+++ string/bits/string2.h	Mon Dec  6 12:18:37 1999
@@ -118,7 +118,7 @@ __STRING2_COPY_TYPE (8);
      __uint8_t __c = (__uint8_t) (c);					      \
 									      \
      /* This `switch' statement will be removed at compile-time.  */	      \
-     switch (n)								      \
+     switch ((unsigned int)n)						      \
        {								      \
        case 15:								      \
 	 __u->__ui = __c * 0x01010101;					      \
@@ -230,7 +230,7 @@ __mempcpy_small (void *__dest1,
     unsigned char __uc;
     unsigned char __c;
   } *__u = __dest1;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__c = __src0_1;
@@ -332,7 +332,7 @@ __mempcpy_small (void *__dest, char __sr
     __STRING2_COPY_ARR7 __sca7;
     __STRING2_COPY_ARR8 __sca8;
   } *__u = __dest;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__c = __src1;
@@ -405,7 +405,7 @@ __strcpy_small (char *__dest,
     __uint16_t __usi;
     unsigned char __uc;
   } *__u = (void *) __dest;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__uc = '\0';
@@ -498,7 +498,7 @@ __strcpy_small (char *__dest,
     __STRING2_COPY_ARR7 __sca7;
     __STRING2_COPY_ARR8 __sca8;
   } *__u = (void *) __dest;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__c = '\0';
@@ -565,7 +565,7 @@ __stpcpy_small (char *__dest,
     unsigned char __uc;
     char __c;
   } *__u = (void *) __dest;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__uc = '\0';
@@ -662,7 +662,7 @@ __stpcpy_small (char *__dest,
     __STRING2_COPY_ARR7 __sca7;
     __STRING2_COPY_ARR8 __sca8;
   } *__u = (void *) __dest;
-  switch (__srclen)
+  switch ((unsigned int)__srclen)
     {
     case 1:
       __u->__c = '\0';

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jakub@redhat.com | http://sunsite.mff.cuni.cz/~jj
Linux version 2.3.18 on a sparc64 machine (1343.49 BogoMips)
___________________________________________________________________

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] shut up warnings in bits/string2.h
  1999-12-07 13:02 [PATCH] shut up warnings in bits/string2.h Jakub Jelinek
@ 1999-12-07 18:41 ` Richard Henderson
  1999-12-07 23:41 ` Ulrich Drepper
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Henderson @ 1999-12-07 18:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: drepper, libc-hacker

On Tue, Dec 07, 1999 at 10:08:11PM +0100, Jakub Jelinek wrote:
> 	* string/bits/string2.h (__memset_gc, __mempcpy_small, __strcpy_small,
> 	__stpcpy_small): Cast switch expressions to int to shut up compiler
> 	warnings.

The patch is wrong Jakub.  Suppose you have a string of
length 0x1_0000_0001.  You will truncate this to 1 and
do the wrong thing.

I know you are trying to silence the extra warnings from
gcc's -Wtraditional, but there's nothing to be done about
it except possibly nuking it from the compiler.


r~

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] shut up warnings in bits/string2.h
  1999-12-07 13:02 [PATCH] shut up warnings in bits/string2.h Jakub Jelinek
  1999-12-07 18:41 ` Richard Henderson
@ 1999-12-07 23:41 ` Ulrich Drepper
  1999-12-08  0:22   ` Jakub Jelinek
  1 sibling, 1 reply; 6+ messages in thread
From: Ulrich Drepper @ 1999-12-07 23:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: libc-hacker

Jakub Jelinek <jakub@redhat.com> writes:

> This patch shuts up gcc 2.96 warnings about long switch expressions not
> converted to int.

Well, rth commented that this might fail for long strings.  But those
strings must be string constants, not actual strings.  And gcc (nor
any other compiler) will have support for strings of more than 2^32
character in length.  So it should be possible to apply the patch.

> How does it work on other architectures?

Which functions?  The generic functions should perform quite good.
The copying of constant strings work by moving immedate values which
is much faster.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] shut up warnings in bits/string2.h
  1999-12-07 23:41 ` Ulrich Drepper
@ 1999-12-08  0:22   ` Jakub Jelinek
  1999-12-08  0:34     ` Ulrich Drepper
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 1999-12-08  0:22 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: libc-hacker

On Tue, Dec 07, 1999 at 11:38:43PM -0800, Ulrich Drepper wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> 
> > This patch shuts up gcc 2.96 warnings about long switch expressions not
> > converted to int.
> 
> Well, rth commented that this might fail for long strings.  But those
> strings must be string constants, not actual strings.  And gcc (nor
> any other compiler) will have support for strings of more than 2^32
> character in length.  So it should be possible to apply the patch.

All the inlines where I changed this are invoked from macros which guard it
by something like this:
__builtin_constant_p (n) && (n) <= 16
where the length limit is 16 or 8 as far as I could see, so I don't see how
that could be a problem (the only problematic thing could be say
memcpy(p, "string", -4) because then n is builtin constant and is <= 16, but
that should be guarded by
__builtin_constant_p (n) && (n) <= 16U
).

> 
> > How does it work on other architectures?
> 
> Which functions?  The generic functions should perform quite good.
> The copying of constant strings work by moving immedate values which
> is much faster.

E.g.
#include <string.h>
void foo(char *p)
{
	strcpy(p, "strn");
}

gets compiled with -O2 -m32 by gcc 2.96 (but similarly by egcs 1.1.2) into:
foo:	save	%sp, -224, %sp
	sethi	%hi(.LLC0+1), %o0
	or	%o0, %lo(.LLC0+1), %o0
	add	%o0, -1, %o5
	sub	%o0, %o5, %o0
	cmp	%o0, 1
	bne	.LL145
	mov	%i0, %o0
	mov	115, %o1
	mov	116, %o2
	mov	114, %o3
	mov	110, %o4
	stb	%o1, [%fp-32]
	stb	%g0, [%fp-31]
	stb	%o1, [%fp-24]
	stb	%g0, [%fp-23]
	stb	%o1, [%fp-48]
	stb	%o2, [%fp-47]
	stb	%g0, [%fp-46]
	stb	%o1, [%fp-40]
	stb	%o2, [%fp-39]
	stb	%g0, [%fp-38]
	stb	%o1, [%fp-64]
	stb	%o2, [%fp-63]
	stb	%o3, [%fp-62]
	stb	%g0, [%fp-61]
	stb	%o1, [%fp-56]
	stb	%o2, [%fp-55]
	stb	%o3, [%fp-54]
	stb	%g0, [%fp-53]
	stb	%o1, [%fp-80]
	stb	%o2, [%fp-79]
	stb	%o3, [%fp-78]
	stb	%o4, [%fp-77]
	stb	%g0, [%fp-76]
	stb	%o1, [%fp-72]
	stb	%o2, [%fp-71]
	stb	%o3, [%fp-70]
	stb	%o4, [%fp-69]
	stb	%g0, [%fp-68]
	ldub	[%o5+5], %o7
	mov	%o1, %l0
	stb	%o1, [%fp-96]
	stb	%o2, [%fp-95]
	stb	%o3, [%fp-94]
	stb	%o4, [%fp-93]
	stb	%g0, [%fp-92]
	stb	%g0, [%fp-91]
	stb	%o1, [%fp-88]
	stb	%o2, [%fp-87]
	stb	%o3, [%fp-86]
	stb	%o4, [%fp-85]
	stb	%g0, [%fp-84]
	stb	%g0, [%fp-83]
	stb	%o1, [%fp-112]
	stb	%o2, [%fp-111]
	stb	%o3, [%fp-110]
	stb	%o4, [%fp-109]
	stb	%g0, [%fp-108]
	stb	%o7, [%fp-107]
	stb	%g0, [%fp-106]
	stb	%o1, [%fp-104]
	stb	%o2, [%fp-103]
	stb	%o3, [%fp-102]
	stb	%o4, [%fp-101]
	stb	%g0, [%fp-100]
	stb	%o7, [%fp-99]
	stb	%g0, [%fp-98]
	ldub	[%o5+6], %l1
	mov	%o2, %g3
	ldub	[%fp-68], %o0
	stb	%o1, [%fp-120]
	stb	%o2, [%fp-119]
	stb	%o3, [%fp-118]
	stb	%o4, [%fp-117]
	stb	%o7, [%fp-115]
	stb	%l1, [%fp-114]
	mov	%o3, %g2
	mov	%o4, %o5
	stb	%o0, [%i0+4]
	stb	%l0, [%i0]
	stb	%g3, [%i0+1]
	stb	%g2, [%i0+2]
	stb	%o5, [%i0+3]
	stb	%o1, [%fp-128]
	stb	%o2, [%fp-127]
	stb	%o3, [%fp-126]
	stb	%o4, [%fp-125]
	stb	%g0, [%fp-124]
	stb	%o7, [%fp-123]
	stb	%l1, [%fp-122]
	stb	%g0, [%fp-121]
	stb	%g0, [%fp-116]
	b	.LL144
	stb	%g0, [%fp-113]
.LL145:	mov	%o5, %o1
	call	memcpy, 0
	mov	5, %o2
.LL144:	ret
	restore

while compiling it with the same compiler with -O2 -D__NO_STRING_INLINES
turns into (and similarly with egcs 1.1.2):
foo:	sethi	%hi(.LLC0), %g2
	ldub	[%g2+%lo(.LLC0)], %g3
	or	%g2, %lo(.LLC0), %g2
	stb	%g3, [%o0]
	ldub	[%g2+1], %g3
	stb	%g3, [%o0+1]
	ldub	[%g2+2], %o1
	stb	%o1, [%o0+2]
	ldub	[%g2+3], %g3
	stb	%g3, [%o0+3]
	ldub	[%g2+4], %o1
	retl
	stb	%o1, [%o0+4]

I haven't yet investigated what's the problem, but some string2.h inlines
work as expected (e.g. memset).

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jakub@redhat.com | http://sunsite.mff.cuni.cz/~jj
Linux version 2.3.18 on a sparc64 machine (1343.49 BogoMips)
___________________________________________________________________

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] shut up warnings in bits/string2.h
  1999-12-08  0:22   ` Jakub Jelinek
@ 1999-12-08  0:34     ` Ulrich Drepper
  1999-12-08  7:56       ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Drepper @ 1999-12-08  0:34 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: libc-hacker

Jakub Jelinek <jakub@redhat.com> writes:

> All the inlines where I changed this are invoked from macros which guard it
> by something like this:
> __builtin_constant_p (n) && (n) <= 16

Yep.

> gets compiled with -O2 -m32 by gcc 2.96 (but similarly by egcs 1.1.2) into:
> foo:	save	%sp, -224, %sp
> 	sethi	%hi(.LLC0+1), %o0
> 	or	%o0, %lo(.LLC0+1), %o0
> 	add	%o0, -1, %o5
> 	sub	%o0, %o5, %o0
> 	cmp	%o0, 1
> 	bne	.LL145
> 	mov	%i0, %o0
> 	mov	115, %o1
> [...]

This certainly is a SPARC problem.  Maybe some other platform, but not
x86 and presumably not m68k.

-- 
---------------.      drepper at gnu.org  ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] shut up warnings in bits/string2.h
  1999-12-08  0:34     ` Ulrich Drepper
@ 1999-12-08  7:56       ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 1999-12-08  7:56 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: libc-hacker

> > gets compiled with -O2 -m32 by gcc 2.96 (but similarly by egcs 1.1.2) into:
> > foo:	save	%sp, -224, %sp
> > 	sethi	%hi(.LLC0+1), %o0
> > 	or	%o0, %lo(.LLC0+1), %o0
> > 	add	%o0, -1, %o5
> > 	sub	%o0, %o5, %o0
> > 	cmp	%o0, 1
> > 	bne	.LL145
> > 	mov	%i0, %o0
> > 	mov	115, %o1
> > [...]
> 
> This certainly is a SPARC problem.  Maybe some other platform, but not
> x86 and presumably not m68k.

Are you sure about this?
If I do on i686:

mkdir bits
touch bits/string.h
cat > test.c <<EOF
#include <string.h>
void foo(char *p) { strcpy(p, "strn"); }
EOF
gcc -S -O2 -o test1.s test.c
gcc -S -O2 -o test2.s test.c -D__NO_STRING_INLINES

test1.s is far longer and slower than test2.s (9 vs. 47 insns).
The fact that you're not normaly seeing it seems that i386 string.h defines
its own versions of the problematic glibc inlines.

BTW: I was posting a bugfix for bits/errno.h yesterday as well, could you
look at it and if it is ok commit it? Thanks.

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jakub@redhat.com | http://sunsite.mff.cuni.cz/~jj
Linux version 2.3.18 on a sparc64 machine (1343.49 BogoMips)
___________________________________________________________________

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~1999-12-08  7:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-12-07 13:02 [PATCH] shut up warnings in bits/string2.h Jakub Jelinek
1999-12-07 18:41 ` Richard Henderson
1999-12-07 23:41 ` Ulrich Drepper
1999-12-08  0:22   ` Jakub Jelinek
1999-12-08  0:34     ` Ulrich Drepper
1999-12-08  7:56       ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).