9fans - fans of the OS Plan 9 from Bell Labs
 help / color / mirror / Atom feed
* [9fans] BUG!!! in Plan9 compiler!
@ 2010-04-22 15:29 tlaronde
  2010-04-22 17:03 ` Bakul Shah
  0 siblings, 1 reply; 20+ messages in thread
From: tlaronde @ 2010-04-22 15:29 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

So I have finally narrow down and compare the calculus under
NetBSD/i386/gcc and Plan9/i386/ken-cc. And this has nothing to do with
div versus shift...

ken-cc is at fault here and it seems deep; it has apparently to deal
with registerization.

Data:
Under NetBSD/gcc, I have the following values:

	before: x1:=5440, x2:=-5843, x3:=78909
	after: x1:=5440, x2:=-201, x3:=18166, r:=6827 t:=30232

Under Plan9/gcc, I have the following values:

	before: x1:=5440, x2:=-5843, x3:=78909
	after: x1:=5440, x2:=2147483447, x3:=1073759990, r:=6827 t:=-1073711592

Uhm... seems to have a `slight' divergence...

In fact, all wrong values depend upon x2, that has the "correct"
value... with 2^31 complement. A positive when it should be negative,
since the offending code is the following:

      x2 = half ( x1 + x2 + xicorr ) ;

	  that is :
		x2 = (5440 - 5843 + 1) / 2;

Not exactly pushing things to the limit! And yes, the expected result is
indeed -201.

In fact, if you just extract this kind of expression, and compile a
test, there is no problem.

But the calling function is teratologic: it has 10 parameters and 18
automatic variables.

Since the problem arises in this context, but not if you just add
this isolated in a test program, and call it with these very 3
values (5440, -5843, 1), it is clear that's the way the computation
is handled with huge number of parameters and auto variables
that wreaks havoc.

If I declare all the auto volatile, this does nothing: same result.

If I do the addition, and afterwards take the half, that works:

x2 += x1 + xicorr;
x2 = half(x2);	/* works! */

Problem: this is not a solution, but a hack, since this kind of 3
variables handling is common. This one fails. And the others?

Is there a limit about the number of parameters?
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 15:29 [9fans] BUG!!! in Plan9 compiler! tlaronde
@ 2010-04-22 17:03 ` Bakul Shah
  2010-04-22 17:36   ` tlaronde
  0 siblings, 1 reply; 20+ messages in thread
From: Bakul Shah @ 2010-04-22 17:03 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, 22 Apr 2010 17:29:53 +0200 tlaronde@polynum.com  wrote:
> Data:
> Under NetBSD/gcc, I have the following values:
>
> 	before: x1:=5440, x2:=-5843, x3:=78909
> 	after: x1:=5440, x2:=-201, x3:=18166, r:=6827 t:=30232
>
> Under Plan9/gcc, I have the following values:
>
> 	before: x1:=5440, x2:=-5843, x3:=78909
> 	after: x1:=5440, x2:=2147483447, x3:=1073759990, r:=6827 t:=-1073711592
>
> Uhm... seems to have a `slight' divergence...
>
> In fact, all wrong values depend upon x2, that has the "correct"
> value... with 2^31 complement. A positive when it should be negative,
> since the offending code is the following:
>
>       x2 = half ( x1 + x2 + xicorr ) ;
>
> 	  that is :
> 		x2 = (5440 - 5843 + 1) / 2;
>
> Not exactly pushing things to the limit! And yes, the expected result is
> indeed -201.

You would get 2147483447 if x1 and x2 were treated as
unsigned numbers but -201 if treated as signed. Try this:

cat > x.c <<EOF
#include <stdio.h>
NUM f(NUM x, NUM y) { return (x + y + 1) / 2; }
int main(int c, char**v) { printf("%d\n", f(atoi(v[1]), atoi(v[2]))); }
EOF
cc -DNUM=signed   x.c && a.out 5440 -5843
cc -DNUM=unsigned x.c && a.out 5440 -5843

What is the type of x1 and x2? Can you show an actual C code
fragment?  Don't worry about it being complete. Just the half()
function (or macro), header of the function where it is
called, declarations for x1 and x2 and a couple of lines of
around call to half. I am still wondering if this is due to a
different interpretation of language semantics by the two
compilers.

> Since the problem arises in this context, but not if you just add
> this isolated in a test program, and call it with these very 3
> values (5440, -5843, 1), it is clear that's the way the computation
> is handled with huge number of parameters and auto variables
> that wreaks havoc.

You *suspect* this but you need to prove it.  An isolated
test case that doesn't trigger this problem simply means you
have not created the right condition for the bug.  Creating a
simple test can be tricky and may be more work than debugging
your program.

> If I declare all the auto volatile, this does nothing: same result.
>
> If I do the addition, and afterwards take the half, that works:
>
> x2 += x1 + xicorr;
> x2 = half(x2);	/* works! */

I wouldn't bother changing anything. You already have a
smoking gun (at least you know in which neighbourhood it has
gone off). You can try a binary search to narrow down the
area but in the end you will have to look at the assembly
output of the relevant code fragment.



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 17:03 ` Bakul Shah
@ 2010-04-22 17:36   ` tlaronde
  2010-04-22 17:50     ` tlaronde
  0 siblings, 1 reply; 20+ messages in thread
From: tlaronde @ 2010-04-22 17:36 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, Apr 22, 2010 at 10:03:58AM -0700, Bakul Shah wrote:
>
> What is the type of x1 and x2? Can you show an actual C code
> fragment?  Don't worry about it being complete. Just the half()

All are "integer" (in WEB -> Pascal ->)

#define integer long

The half() macro is only this:

#define half(x)	((x) >> 1)

(Changing it to (x)/2 changes nothing: I have tried).

All are signed and long type (i.e. for 32 bits). This is scaled integers
calculus.

And if you want the full story, here is the listing generated (this is
from the translator, so it is from a style point of view, ugly;
specially tracking what is a block for if or else is not obvious from
sight. But it is short. I have put a comment on the branch where the
problem arises):

#define integer long
typedef integer scaled;
#define half(x)	((x) >> 1)

void makemoves ( scaled xx0 , scaled xx1 , scaled xx2 , scaled xx3 , scaled
yy0 , scaled yy1 , scaled yy2 , scaled yy3 , smallnumber xicorr , smallnumber
etacorr )
{/* 22 30 10 */ integer x1, x2, x3, m, r, y1, y2, y3, n, s, l  ;
  integer q, t, u, x2a, x3a, y2a, y3a  ;
  if ( ( xx3 < xx0 ) || ( yy3 < yy0 ) )
  confusion ( 109 ) ;
  l = 16 ;
  bisectptr = 0 ;
  x1 = xx1 - xx0 ;
  x2 = xx2 - xx1 ;
  x3 = xx3 - xx2 ;
  if ( xx0 >= xicorr )
  r = ( xx0 - xicorr ) % 65536L ;
  else r = 65535L - ( ( - (integer) xx0 + xicorr - 1 ) % 65536L ) ;
  m = ( xx3 - xx0 + r ) / 65536L ;
  y1 = yy1 - yy0 ;
  y2 = yy2 - yy1 ;
  y3 = yy3 - yy2 ;
  if ( yy0 >= etacorr )
  s = ( yy0 - etacorr ) % 65536L ;
  else s = 65535L - ( ( - (integer) yy0 + etacorr - 1 ) % 65536L ) ;
  n = ( yy3 - yy0 + s ) / 65536L ;
  if ( ( xx3 - xx0 >= 268435456L ) || ( yy3 - yy0 >= 268435456L ) )
  {
    x1 = half ( x1 + xicorr ) ;
    x2 = half ( x2 + xicorr ) ;
    x3 = half ( x3 + xicorr ) ;
    r = half ( r + xicorr ) ;
    y1 = half ( y1 + etacorr ) ;
    y2 = half ( y2 + etacorr ) ;
    y3 = half ( y3 + etacorr ) ;
    s = half ( s + etacorr ) ;
    l = 15 ;
  }
  while ( true ) {

    lab22: if ( m == 0 )
    while ( n > 0 ) {

      incr ( moveptr ) ;
      move [ moveptr ] = 1 ;
      decr ( n ) ;
    }
    else if ( n == 0 )
    move [ moveptr ] = move [ moveptr ] + m ;
    else if ( m + n == 2 )
    {
      r = twotothe [ l ] - r ;
      s = twotothe [ l ] - s ;
      while ( l < 30 ) {

	x3a = x3 ;
	x2a = half ( x2 + x3 + xicorr ) ;
	x2 = half ( x1 + x2 + xicorr ) ;
	x3 = half ( x2 + x2a + xicorr ) ;
	t = x1 + x2 + x3 ;
	r = r + r - xicorr ;
	y3a = y3 ;
	y2a = half ( y2 + y3 + etacorr ) ;
	y2 = half ( y1 + y2 + etacorr ) ;
	y3 = half ( y2 + y2a + etacorr ) ;
	u = y1 + y2 + y3 ;
	s = s + s - etacorr ;
	if ( t < r )
	if ( u < s )
	{
	  x1 = x3 ;
	  x2 = x2a ;
	  x3 = x3a ;
	  r = r - t ;
	  y1 = y3 ;
	  y2 = y2a ;
	  y3 = y3a ;
	  s = s - u ;
	}
	else {

	  {
	    incr ( moveptr ) ;
	    move [ moveptr ] = 2 ;
	  }
	  goto lab30 ;
	}
	else if ( u < s )
	{
	  {
	    incr ( move [ moveptr ] ) ;
	    incr ( moveptr ) ;
	    move [ moveptr ] = 1 ;
	  }
	  goto lab30 ;
	}
	incr ( l ) ;
      }
      r = r - xicorr ;
      s = s - etacorr ;
      if ( abvscd ( x1 + x2 + x3 , s , y1 + y2 + y3 , r ) - xicorr >= 0 )
      {
	incr ( move [ moveptr ] ) ;
	incr ( moveptr ) ;
	move [ moveptr ] = 1 ;
      }
      else {

	incr ( moveptr ) ;
	move [ moveptr ] = 2 ;
      }
      lab30: ;
    }
    else { /* This is where the problem arises */

      incr ( l ) ;
      bisectstack [ bisectptr + 10 ] = l ;
      bisectstack [ bisectptr + 2 ] = x3 ;
      bisectstack [ bisectptr + 1 ] = half ( x2 + x3 + xicorr ) ;
      x2 = half ( x1 + x2 + xicorr ) ;
      x3 = half ( x2 + bisectstack [ bisectptr + 1 ] + xicorr ) ;
      bisectstack [ bisectptr ] = x3 ;
      r = r + r + xicorr ;
      t = x1 + x2 + x3 + r ;
      q = bdiv ( t , l ) ;
      bisectstack [ bisectptr + 3 ] = bmod ( t , l ) ;
      bisectstack [ bisectptr + 4 ] = m - q ;
      m = q ;
      bisectstack [ bisectptr + 7 ] = y3 ;
      bisectstack [ bisectptr + 6 ] = half ( y2 + y3 + etacorr ) ;
      y2 = half ( y1 + y2 + etacorr ) ;
      y3 = half ( y2 + bisectstack [ bisectptr + 6 ] + etacorr ) ;
      bisectstack [ bisectptr + 5 ] = y3 ;
      s = s + s + etacorr ;
      u = y1 + y2 + y3 + s ;
      q = bdiv ( u , l ) ;
      bisectstack [ bisectptr + 8 ] = bmod ( u , l ) ;
      bisectstack [ bisectptr + 9 ] = n - q ;
      n = q ;
      bisectptr = bisectptr + 11 ;
      goto lab22 ;
    }
    if ( bisectptr == 0 )
    goto lab10 ;
    bisectptr = bisectptr - 11 ;
    x1 = bisectstack [ bisectptr ] ;
    x2 = bisectstack [ bisectptr + 1 ] ;
    x3 = bisectstack [ bisectptr + 2 ] ;
    r = bisectstack [ bisectptr + 3 ] ;
    m = bisectstack [ bisectptr + 4 ] ;
    y1 = bisectstack [ bisectptr + 5 ] ;
    y2 = bisectstack [ bisectptr + 6 ] ;
    y3 = bisectstack [ bisectptr + 7 ] ;
    s = bisectstack [ bisectptr + 8 ] ;
    n = bisectstack [ bisectptr + 9 ] ;
    l = bisectstack [ bisectptr + 10 ] ;
  }
  lab10: ;
}
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 17:36   ` tlaronde
@ 2010-04-22 17:50     ` tlaronde
  2010-04-22 19:08       ` geoff
  0 siblings, 1 reply; 20+ messages in thread
From: tlaronde @ 2010-04-22 17:50 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

Oups, I sent the version with the modifications applied (enrolling).
Here is the vanilla version (with reformating):

void
makemoves( scaled xx0 , scaled xx1 , scaled xx2 , scaled xx3 ,
	scaled yy0 , scaled yy1 , scaled yy2 , scaled yy3 ,
	smallnumber xicorr , smallnumber etacorr )
{/* 22 30 10 */ integer x1, x2, x3, m, r, y1, y2, y3, n, s, l  ;
  integer q, t, u, x2a, x3a, y2a, y3a  ;

  if ( ( xx3 < xx0 ) || ( yy3 < yy0 ) )
  	confusion ( 109 ) ;
  l = 16 ;
  bisectptr = 0 ;
  x1 = xx1 - xx0 ;
  x2 = xx2 - xx1 ;
  x3 = xx3 - xx2 ;
  if ( xx0 >= xicorr )
  	r = ( xx0 - xicorr ) % 65536L ;
  else
	r = 65535L - ( ( - (integer) xx0 + xicorr - 1 ) % 65536L ) ;

  m = ( xx3 - xx0 + r ) / 65536L ;
  y1 = yy1 - yy0 ;
  y2 = yy2 - yy1 ;
  y3 = yy3 - yy2 ;
  if ( yy0 >= etacorr )
  	s = ( yy0 - etacorr ) % 65536L ;
  else
	s = 65535L - ( ( - (integer) yy0 + etacorr - 1 ) % 65536L ) ;
  n = ( yy3 - yy0 + s ) / 65536L ;

  if ( ( xx3 - xx0 >= 268435456L ) || ( yy3 - yy0 >= 268435456L ) ) {
		x1 = half ( x1 + xicorr ) ;
		x2 = half ( x2 + xicorr ) ;
		x3 = half ( x3 + xicorr ) ;
		r = half ( r + xicorr ) ;
		y1 = half ( y1 + etacorr ) ;
		y2 = half ( y2 + etacorr ) ;
		y3 = half ( y3 + etacorr ) ;
		s = half ( s + etacorr ) ;
		l = 15 ;
  }

  while ( true ) {
    lab22:
	if ( m == 0 ) {
		while ( n > 0 ) {
			incr ( moveptr ) ;
			move [ moveptr ] = 1 ;
			decr ( n ) ;
		}
	} else if ( n == 0 )
   		move [ moveptr ] = move [ moveptr ] + m ;
   	else if ( m + n == 2 ) {
			r = twotothe [ l ] - r ;
			s = twotothe [ l ] - s ;
			while ( l < 30 ) {
				x3a = x3 ;
				x2a = half ( x2 + x3 + xicorr ) ;
				x2 = half ( x1 + x2 + xicorr ) ;
				x3 = half ( x2 + x2a + xicorr ) ;
				t = x1 + x2 + x3 ;
				r = r + r - xicorr ;
				y3a = y3 ;
				y2a = half ( y2 + y3 + etacorr ) ;
				y2 = half ( y1 + y2 + etacorr ) ;
				y3 = half ( y2 + y2a + etacorr ) ;
				u = y1 + y2 + y3 ;
				s = s + s - etacorr ;
				if ( t < r ) {
					if ( u < s ) {
					  x1 = x3 ;
					  x2 = x2a ;
					  x3 = x3a ;
					  r = r - t ;
					  y1 = y3 ;
					  y2 = y2a ;
					  y3 = y3a ;
					  s = s - u ;
					}
				} else {
					  {
					    incr ( moveptr ) ;
					    move [ moveptr ] = 2 ;
					  }
					  goto lab30 ;
				} else if ( u < s ) {
					  {
					    incr ( move [ moveptr ] ) ;
					    incr ( moveptr ) ;
					    move [ moveptr ] = 1 ;
					  }
					  goto lab30 ;
				}
				incr ( l ) ;
			}  /* end while */
			r = r - xicorr ;
			s = s - etacorr ;
			if ( abvscd ( x1 + x2 + x3 , s , y1 + y2 + y3 , r ) - xicorr >= 0 ) {
				incr ( move [ moveptr ] ) ;
				incr ( moveptr ) ;
				move [ moveptr ] = 1 ;
			} else {
					incr ( moveptr ) ;
					move [ moveptr ] = 2 ;
				}
			lab30: ;
	} else {	/* The problem arises in this branch */
      incr ( l ) ;
      bisectstack [ bisectptr + 10 ] = l ;
      bisectstack [ bisectptr + 2 ] = x3 ;
      bisectstack [ bisectptr + 1 ] = half ( x2 + x3 + xicorr ) ;
      x2 = half ( x1 + x2 + xicorr ) ;
      x3 = half ( x2 + bisectstack [ bisectptr + 1 ] + xicorr ) ;
      bisectstack [ bisectptr ] = x3 ;
      r = r + r + xicorr ;
      t = x1 + x2 + x3 + r ;
      q = t / twotothe [ l ] ;
      bisectstack [ bisectptr + 3 ] = t % twotothe [ l ] ;
      bisectstack [ bisectptr + 4 ] = m - q ;
      m = q ;
      bisectstack [ bisectptr + 7 ] = y3 ;
      bisectstack [ bisectptr + 6 ] = half ( y2 + y3 + etacorr ) ;
      y2 = half ( y1 + y2 + etacorr ) ;
      y3 = half ( y2 + bisectstack [ bisectptr + 6 ] + etacorr ) ;
      bisectstack [ bisectptr + 5 ] = y3 ;
      s = s + s + etacorr ;
      u = y1 + y2 + y3 + s ;
      q = u / twotothe [ l ] ;
      bisectstack [ bisectptr + 8 ] = u % twotothe [ l ] ;
      bisectstack [ bisectptr + 9 ] = n - q ;
      n = q ;
      bisectptr = bisectptr + 11 ;
      goto lab22 ;
    }
    if ( bisectptr == 0 )
    	goto lab10 ;
    bisectptr = bisectptr - 11 ;
    x1 = bisectstack [ bisectptr ] ;
    x2 = bisectstack [ bisectptr + 1 ] ;
    x3 = bisectstack [ bisectptr + 2 ] ;
    r = bisectstack [ bisectptr + 3 ] ;
    m = bisectstack [ bisectptr + 4 ] ;
    y1 = bisectstack [ bisectptr + 5 ] ;
    y2 = bisectstack [ bisectptr + 6 ] ;
    y3 = bisectstack [ bisectptr + 7 ] ;
    s = bisectstack [ bisectptr + 8 ] ;
    n = bisectstack [ bisectptr + 9 ] ;
    l = bisectstack [ bisectptr + 10 ] ;
  } /* end true */
  lab10: ;
}
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 17:50     ` tlaronde
@ 2010-04-22 19:08       ` geoff
  2010-04-22 19:32         ` tlaronde
  0 siblings, 1 reply; 20+ messages in thread
From: geoff @ 2010-04-22 19:08 UTC (permalink / raw)
  To: 9fans

What type is `smallnumber'?



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 19:08       ` geoff
@ 2010-04-22 19:32         ` tlaronde
  2010-04-22 20:07           ` Bakul Shah
  0 siblings, 1 reply; 20+ messages in thread
From: tlaronde @ 2010-04-22 19:32 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, Apr 22, 2010 at 03:08:40PM -0400, geoff@plan9.bell-labs.com wrote:
> What type is `smallnumber'?

typedef unsigned char smallnumber;

translated from Pascal:

small_number=0..63;

--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 19:32         ` tlaronde
@ 2010-04-22 20:07           ` Bakul Shah
  2010-04-22 21:15             ` tlaronde
  0 siblings, 1 reply; 20+ messages in thread
From: Bakul Shah @ 2010-04-22 20:07 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, 22 Apr 2010 21:32:36 +0200 tlaronde@polynum.com  wrote:
> On Thu, Apr 22, 2010 at 03:08:40PM -0400, geoff@plan9.bell-labs.com wrote:
> > What type is `smallnumber'?
>
> typedef unsigned char smallnumber;
          ^^^^^^^^
Aha!

> translated from Pascal:
>
> small_number=0..63;

IIRC in C89 integer promotions rules changed.  See 6.3.1.8
(Usual arithmetic conversions)

            [Otherwise,]the integer promotions are performed on both
            operands.   Then the following rules are applied to the
            promoted operands:

                 If both operands  have  the  same  type,  then  no
                 further conversion is needed.

                 Otherwise,  if  both  operands have signed integer
                 types or both have  unsigned  integer  types,  the
                 operand with the type of lesser integer conversion
                 rank is converted to the type of the operand  with
                 greater rank.

                 Otherwise,   if  the  operand  that  has  unsigned
                 integer type has rank greater or equal to the rank
                 of the type of the other operand, then the operand
                 with signed integer type is converted to the  type
                 of the operand with unsigned integer type.

>>>>             Otherwise,  if the type of the operand with signed
                 integer type can represent all of  the  values  of
                 the  type  of  the  operand  with unsigned integer
                 type, then the operand with unsigned integer  type
                 is  converted  to  the  type  of  the operand with
                 signed integer type.

                 Otherwise, both  operands  are  converted  to  the
                 unsigned integer type corresponding to the type of
                 the operand with signed integer type.

Try this on both gcc and 8c (with suitable changes):

#define N(i) atoi(v[i])
int f(int x, int y, unsigned char z) { return (x + y + z) / 2; }
int main(int c, char**v) { printf("%d\n", f(N(1), N(2), N(3))); }

And see what you get.

I suspect you can safely change smallnumber to a signed char
and most likely this particular problem will go away.



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 20:07           ` Bakul Shah
@ 2010-04-22 21:15             ` tlaronde
  2010-04-22 21:26               ` tlaronde
  2010-04-22 22:49               ` Bakul Shah
  0 siblings, 2 replies; 20+ messages in thread
From: tlaronde @ 2010-04-22 21:15 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

Bingo! see below...

On Thu, Apr 22, 2010 at 01:07:40PM -0700, Bakul Shah wrote:
> On Thu, 22 Apr 2010 21:32:36 +0200 tlaronde@polynum.com  wrote:
> > On Thu, Apr 22, 2010 at 03:08:40PM -0400, geoff@plan9.bell-labs.com wrote:
> > > What type is `smallnumber'?
> >
> > typedef unsigned char smallnumber;
>           ^^^^^^^^
> Aha!
>
> > translated from Pascal:
> >
> > small_number=0..63;
>
> IIRC in C89 integer promotions rules changed.  See 6.3.1.8
> (Usual arithmetic conversions)
>
>             [Otherwise,]the integer promotions are performed on both
>             operands.   Then the following rules are applied to the
>             promoted operands:
>
>                  If both operands  have  the  same  type,  then  no
>                  further conversion is needed.
>
>                  Otherwise,  if  both  operands have signed integer
>                  types or both have  unsigned  integer  types,  the
>                  operand with the type of lesser integer conversion
>                  rank is converted to the type of the operand  with
>                  greater rank.
>
>                  Otherwise,   if  the  operand  that  has  unsigned
>                  integer type has rank greater or equal to the rank
>                  of the type of the other operand, then the operand
>                  with signed integer type is converted to the  type
>                  of the operand with unsigned integer type.
>
> >>>>             Otherwise,  if the type of the operand with signed
>                  integer type can represent all of  the  values  of
>                  the  type  of  the  operand  with unsigned integer
>                  type, then the operand with unsigned integer  type
>                  is  converted  to  the  type  of  the operand with
>                  signed integer type.
>
>                  Otherwise, both  operands  are  converted  to  the
>                  unsigned integer type corresponding to the type of
>                  the operand with signed integer type.
>
> Try this on both gcc and 8c (with suitable changes):
>
> #define N(i) atoi(v[i])
> int f(int x, int y, unsigned char z) { return (x + y + z) / 2; }
> int main(int c, char**v) { printf("%d\n", f(N(1), N(2), N(3))); }

I get:

gcc: -201
ken-cc: 2147483447 (2^31 - 201)

This is: signed long + signed long + unsigned char.

Do you mean that there is first promotion :

	1) unsigned char is promoted to unsigned int (A6.1).

	2) And since there is an arithmetic operator (/ or shift),
	unsigned int value may exceeds signed long == signed int, the
	signed long is converted to unsigned long (A6.2)?

Is it this?!!!!?

And when I do first assignment, there is only promotion (since no
operator is here). Yielding the correct value in x2, that is then
divided (or shifted) by 2, hence signed, and no problem?

Well, thanks for the lessons, yesterday, today and tomorrow...;)
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 21:15             ` tlaronde
@ 2010-04-22 21:26               ` tlaronde
  2010-04-22 22:49               ` Bakul Shah
  1 sibling, 0 replies; 20+ messages in thread
From: tlaronde @ 2010-04-22 21:26 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, Apr 22, 2010 at 11:15:51PM +0200, tlaronde@polynum.com wrote:
>
> I get:
>
> gcc: -201
> ken-cc: 2147483447 (2^31 - 201)
>
> This is: signed long + signed long + unsigned char.
>
> Do you mean that there is first promotion :
>
> 	1) unsigned char is promoted to unsigned int (A6.1).
>
> 	2) And since there is an arithmetic operator (/ or shift),
> 	unsigned int value may exceeds signed long == signed int, the
> 	signed long is converted to unsigned long (A6.2)?
>
> Is it this?!!!!?
>
> And when I do first assignment, there is only promotion (since no
> operator is here). Yielding the correct value in x2, that is then
> divided (or shifted) by 2, hence signed, and no problem?

I say non-sense: even in the assignment, there is still arithmetic
conversions: the '+'...

I don't get it...
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 21:15             ` tlaronde
  2010-04-22 21:26               ` tlaronde
@ 2010-04-22 22:49               ` Bakul Shah
  2010-04-23  7:42                 ` tlaronde
  2010-04-23 18:53                 ` C H Forsyth
  1 sibling, 2 replies; 20+ messages in thread
From: Bakul Shah @ 2010-04-22 22:49 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, 22 Apr 2010 23:15:51 +0200 tlaronde@polynum.com  wrote:
> This is: signed long + signed long + unsigned char.

> Do you mean that there is first promotion :
>
> 	1) unsigned char is promoted to unsigned int (A6.1).

As per C89 in this case the unsigned char value should be
promoted to a *signed* int value.  The sum will be of type
signed int and so the division will do the right thing. In
kencc case it seems the sum has type unsigned int for some
reason and further, the signed divisor (2) is promoted to an
unsigned int. Seems like a bug.

Now that you know the problem, you can work around it by
setting type smallnumber to a signed char (since its range is
0..64 this should just work with either compiler).

> And when I do first assignment, there is only promotion (since no
> operator is here).

There is promotion since you did += but it doesn't matter.
In C, a variable has a static type and you can't override
this type by any assignment.

> Yielding the correct value in x2, that is then
> divided (or shifted) by 2, hence signed, and no problem?

Yes.



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 22:49               ` Bakul Shah
@ 2010-04-23  7:42                 ` tlaronde
  2010-04-23 18:53                 ` C H Forsyth
  1 sibling, 0 replies; 20+ messages in thread
From: tlaronde @ 2010-04-23  7:42 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Thu, Apr 22, 2010 at 03:49:11PM -0700, Bakul Shah wrote:
> On Thu, 22 Apr 2010 23:15:51 +0200 tlaronde@polynum.com  wrote:
> > This is: signed long + signed long + unsigned char.
>
> > Do you mean that there is first promotion :
> >
> > 	1) unsigned char is promoted to unsigned int (A6.1).
>
> As per C89 in this case the unsigned char value should be
> promoted to a *signed* int value.  The sum will be of type
> signed int and so the division will do the right thing. In
> kencc case it seems the sum has type unsigned int for some
> reason and further, the signed divisor (2) is promoted to an
> unsigned int. Seems like a bug.
>
> Now that you know the problem, you can work around it by
> setting type smallnumber to a signed char (since its range is
> 0..64 this should just work with either compiler).
>
> > And when I do first assignment, there is only promotion (since no
> > operator is here).
>
> There is promotion since you did += but it doesn't matter.
> In C, a variable has a static type and you can't override
> this type by any assignment.
>
> > Yielding the correct value in x2, that is then
> > divided (or shifted) by 2, hence signed, and no problem?
>
> Yes.

OK, thanks for the clarifications---more conform to what I
expected/the way I interpreted the norm---.

You gave a very simple test case for verifying a fix.

Cheers,
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 18:53                 ` C H Forsyth
@ 2010-04-23 18:51                   ` tlaronde
  2010-04-23 20:08                   ` Bakul Shah
  1 sibling, 0 replies; 20+ messages in thread
From: tlaronde @ 2010-04-23 18:51 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Fri, Apr 23, 2010 at 07:53:24PM +0100, C H Forsyth wrote:
>
> on 19 April, I wrote:
> >notably the compiler doesn't implement the value-preserving rules for comparisons. ...
> >instead the compiler implements the older unsigned-preserving rule

It's a chance I was working with a software as described and as
explained as D.E. Knuth's METAFONT, with test suite.

Because without that, and with "erratic" results, I would probably
have dropped the ball...

Hopefully now, everybody knows that this is not only a "theorical"
divergence, but might have rather actual surprising results!
--
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                      http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-22 22:49               ` Bakul Shah
  2010-04-23  7:42                 ` tlaronde
@ 2010-04-23 18:53                 ` C H Forsyth
  2010-04-23 18:51                   ` tlaronde
  2010-04-23 20:08                   ` Bakul Shah
  1 sibling, 2 replies; 20+ messages in thread
From: C H Forsyth @ 2010-04-23 18:53 UTC (permalink / raw)
  To: 9fans

>As per C89 in this case the unsigned char value should be
>promoted to a *signed* int value.  The sum will be of type
>signed int and so the division will do the right thing. In
>kencc case it seems the sum has type unsigned int for some
>reason and further, the signed divisor (2) is promoted to an
>unsigned int. Seems like a bug.

on 19 April, I wrote:
>notably the compiler doesn't implement the value-preserving rules for comparisons. ...
>instead the compiler implements the older unsigned-preserving rule



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 18:53                 ` C H Forsyth
  2010-04-23 18:51                   ` tlaronde
@ 2010-04-23 20:08                   ` Bakul Shah
  2010-04-23 20:46                     ` ron minnich
  1 sibling, 1 reply; 20+ messages in thread
From: Bakul Shah @ 2010-04-23 20:08 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Fri, 23 Apr 2010 19:53:24 BST C H Forsyth <forsyth@vitanuova.com>  wrote:
> >As per C89 in this case the unsigned char value should be
> >promoted to a *signed* int value.  The sum will be of type
> >signed int and so the division will do the right thing. In
> >kencc case it seems the sum has type unsigned int for some
> >reason and further, the signed divisor (2) is promoted to an
> >unsigned int. Seems like a bug.
>
> on 19 April, I wrote:
> >notably the compiler doesn't implement the value-preserving rules for compar
> isons. ...
> >instead the compiler implements the older unsigned-preserving rule

Does the following assertion fail under plan9?

    int x = 1234, y = -4321;
    unsigned char z = 12;
    int r1, r2;

    r1 = (x + y + z)/2;

    x += z;
    r2 = (x + y)/2;

    assert(r1 == r2);

If so, I consider it a bug; particularly as there is no
overflow involved anywhere.



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 20:08                   ` Bakul Shah
@ 2010-04-23 20:46                     ` ron minnich
  2010-04-23 21:44                       ` erik quanstrom
  2010-04-23 22:34                       ` erik quanstrom
  0 siblings, 2 replies; 20+ messages in thread
From: ron minnich @ 2010-04-23 20:46 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Fri, Apr 23, 2010 at 1:08 PM, Bakul Shah <bakul+plan9@bitblocks.com> wrote:

>
> If so, I consider it a bug; particularly as there is no
> overflow involved anywhere.
>
>

r1 2147482110 r2 -1537
ron



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 20:46                     ` ron minnich
@ 2010-04-23 21:44                       ` erik quanstrom
  2010-04-23 22:34                       ` erik quanstrom
  1 sibling, 0 replies; 20+ messages in thread
From: erik quanstrom @ 2010-04-23 21:44 UTC (permalink / raw)
  To: 9fans

> On Fri, Apr 23, 2010 at 1:08 PM, Bakul Shah <bakul+plan9@bitblocks.com> wrote:
>
> >
> > If so, I consider it a bug; particularly as there is no
> > overflow involved anywhere.
> >
> >
>
> r1 2147482110 r2 -1537

that's be 0 for 2.  the expected answer is 1543.
same answer on arm.

acid: (1234 - 4321)/2\D
-1543

- erik



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 20:46                     ` ron minnich
  2010-04-23 21:44                       ` erik quanstrom
@ 2010-04-23 22:34                       ` erik quanstrom
  2010-04-24 18:59                         ` Bakul Shah
  1 sibling, 1 reply; 20+ messages in thread
From: erik quanstrom @ 2010-04-23 22:34 UTC (permalink / raw)
  To: 9fans

> On Fri, Apr 23, 2010 at 1:08 PM, Bakul Shah <bakul+plan9@bitblocks.com> wrote:
>
> >
> > If so, I consider it a bug; particularly as there is no
> > overflow involved anywhere.
> >
> >
>
> r1 2147482110 r2 -1537

on arm the difference is interesting.  the first
/ is translated:
	main+0x20 0x00001040	MOVW	(R4>>1),R4
and the second:
	main+0x34 0x00001054	ADD.MI	$#0x1,R2,R2
	main+0x38 0x00001058	MOVW	(R2->1),R2

(5c -S has it SRL and SRA, respectively.)
if we are unsigned preserving, wouldn't this make sense, since
(x + y + z) is an unsigned expression but (x + y) is not.

- erik



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-23 22:34                       ` erik quanstrom
@ 2010-04-24 18:59                         ` Bakul Shah
  2010-04-24 21:47                           ` Charles Forsyth
  2010-04-25  0:31                           ` erik quanstrom
  0 siblings, 2 replies; 20+ messages in thread
From: Bakul Shah @ 2010-04-24 18:59 UTC (permalink / raw)
  To: Fans of the OS Plan 9 from Bell Labs

On Fri, 23 Apr 2010 18:34:56 EDT erik quanstrom <quanstro@labs.coraid.com>  wrote:
> > On Fri, Apr 23, 2010 at 1:08 PM, Bakul Shah <bakul+plan9@bitblocks.com> wro
> te:
> >
> > >
> > > If so, I consider it a bug; particularly as there is no
> > > overflow involved anywhere.
> > >
> > >
> >
> > r1 2147482110 r2 -1537
>
> on arm the difference is interesting.  the first
> / is translated:
> 	main+0x20 0x00001040	MOVW	(R4>>1),R4
> and the second:
> 	main+0x34 0x00001054	ADD.MI	$#0x1,R2,R2
> 	main+0x38 0x00001058	MOVW	(R2->1),R2
>
> (5c -S has it SRL and SRA, respectively.)
> if we are unsigned preserving, wouldn't this make sense, since
> (x + y + z) is an unsigned expression but (x + y) is not.

It is clear that there is no one *right* answer for how mixed
signed/unsigned number operations should be treated but c89
rules do seem a bit more sensible to me as they avoid
surprises like (x + y + 0u)/z being different from (x + y)/z,
where x,y,z are signed ints.

Is this behaviour really useful for anything? Is there
anything in plan9 code that relies on this behaviour in a
critical way? I suspect this rule can be changed without
impacting plan 9 code much (which as a rule is of much higher
quality than most open source code) and we already know
programs ported to p9p work fine.

Anyway, I think I've said all I want to on this topic!
Probably more than.



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-24 18:59                         ` Bakul Shah
@ 2010-04-24 21:47                           ` Charles Forsyth
  2010-04-25  0:31                           ` erik quanstrom
  1 sibling, 0 replies; 20+ messages in thread
From: Charles Forsyth @ 2010-04-24 21:47 UTC (permalink / raw)
  To: 9fans

>but c89 rules do seem a bit more sensible to me as they avoid surprises

have you read the rules?



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

* Re: [9fans] BUG!!! in Plan9 compiler!
  2010-04-24 18:59                         ` Bakul Shah
  2010-04-24 21:47                           ` Charles Forsyth
@ 2010-04-25  0:31                           ` erik quanstrom
  1 sibling, 0 replies; 20+ messages in thread
From: erik quanstrom @ 2010-04-25  0:31 UTC (permalink / raw)
  To: 9fans

> Is this behaviour really useful for anything? Is there
> anything in plan9 code that relies on this behaviour in a
> critical way? I suspect this rule can be changed without
> impacting plan 9 code much (which as a rule is of much higher
> quality than most open source code) and we already know
> programs ported to p9p work fine.

the network stack has a few bits that don't work without
changes, as i painfully learned with 9vx.

if a patch to the c compiler showed up that did this and
were distributed with sources, i would only consider using
it only if there were an automatic way of locating code that
might change behavior with a lowish false positive rate and
a verifiably zero false negative rate.  even then it would be
very painful as i would need to verify that the system and
all our shipping code works properly with new rules.  i see
little upside.

in short, That Boat Has Sailed.

- erik



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

end of thread, other threads:[~2010-04-25  0:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-22 15:29 [9fans] BUG!!! in Plan9 compiler! tlaronde
2010-04-22 17:03 ` Bakul Shah
2010-04-22 17:36   ` tlaronde
2010-04-22 17:50     ` tlaronde
2010-04-22 19:08       ` geoff
2010-04-22 19:32         ` tlaronde
2010-04-22 20:07           ` Bakul Shah
2010-04-22 21:15             ` tlaronde
2010-04-22 21:26               ` tlaronde
2010-04-22 22:49               ` Bakul Shah
2010-04-23  7:42                 ` tlaronde
2010-04-23 18:53                 ` C H Forsyth
2010-04-23 18:51                   ` tlaronde
2010-04-23 20:08                   ` Bakul Shah
2010-04-23 20:46                     ` ron minnich
2010-04-23 21:44                       ` erik quanstrom
2010-04-23 22:34                       ` erik quanstrom
2010-04-24 18:59                         ` Bakul Shah
2010-04-24 21:47                           ` Charles Forsyth
2010-04-25  0:31                           ` erik quanstrom

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 NNTP newsgroup(s).