Bill Allombert on Wed, 2 Oct 2002 20:54:01 +0200


[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]

Re: functions returning int with 'l' protocodes.


On Wed, Oct 02, 2002 at 08:21:31PM +0200, Karim BELABAS wrote:
> On Tue, 10 Sep 2002, Bill Allombert wrote:
> > Comparison functions usually return an `int' not a `long'.
> > This is correct since they always returns 0, 1 or -1.
> >
> > However some of them are available from GP, but there is
> > not protoype letter for int, so we use l instead.
> >
> > But this is wrong, since on 64bit mechines, int is 32 bit and
> > long 64bit.
> >
> > On ia64 PARI compiled with gcc -g (for all versions) do not pass
> > the bench, because in some cases, (int) -1 became (long) 2^32-1,
> > which is correct, since we have hid the cast from the compiler.
> >
> > FIx involves either add a new letter code for integers, or
> > `upgrade' all functions return int to long.
> > The second option seems better.
> 
> Why not tell the compiler about the cast ?

We can't. We have not made the information available for GP.
GP just don't know this function return a int instead of a long!

> I.e, does the following patch fix the problem ?
> 
> Index: src/language/anal.c
> ===================================================================
> RCS file: /home/megrez/cvsroot/pari/src/language/anal.c,v
> retrieving revision 1.116
> diff -u -b -r1.116 anal.c
> --- src/language/anal.c 2002/09/15 15:43:45     1.116
> +++ src/language/anal.c 2002/10/02 18:19:54
> @@ -1947,7 +1947,7 @@
>         break;
> 
>        case RET_INT:
> -       m = ((long (*)(ANYARG))call)(_ARGS_);
> +       m = (long) ((long (*)(ANYARG))call)(_ARGS_);
>         res = stoi(m); break;
> 
>        case RET_VOID:

This patch is a no-op. You are casting a long  to a long.

In fact the compiler has no way to know the type of the function pointed by
'call' which is an aonymous pointer.

What happen in practice is that the offending function return a 64bit value
in the return register of which only the low  32 bits are meaningful.
It is the responsibility of the caller to expand them to a full 64 bit value.

In fact it is even stranger. (Thanks to Vincent Lefevre who help me to
understand the ia64 asm code generated by gcc).
With optimisation enabled, the value is put expanded directly in the return
register, so there is no problem.
Without optimisation,  the value is put in a register then written to the stack
(but only the low 32 bits) then reread to the return register (again only the
low 32 bits), so finally only the low 32 bits in the return register are correct.


a correct patch would be (changing RET_INT to RET_LONG for proper naming)

      case RET_INT:
        m = ((int (*)(ANYARG))call)(_ARGS_);
        res = stoi(m); break;

      case RET_LONG:
        m = ((long (*)(ANYARG))call)(_ARGS_);
        res = stoi(m); break;

      case RET_VOID:
        ((void (*)(ANYARG))call)(_ARGS_);
        res = gnil; break;
    }

I am not able to find a solution to fix this bug in the 2.1 serie without breaking 
either the API or the ABI. Sad.

Cheers,
Bill.