chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.79k stars 421 forks source link

signbit() - simplification #24578

Open damianmoz opened 8 months ago

damianmoz commented 8 months ago

Can I suggest using raw Chapel with the transmute() feature:

inline proc signbit(param x : real(?w)) param
{
    return (x.transmute(uint(w))) >> (w - 1);
}
inline proc signbit(const x : real(?w))
{
    return (x.transmute(uint(w))) >> (w - 1);
}

A lot cleaner and simpler that what exists. You can even use do to shorten these.

A bit is a uint(1) if such a thing existed. Unless I am missing something, it is not a bool. Current practice has the proc returning a bool result which means an extra operation would be needed during the routine. And if I want it as a bit, then I need to cast it back again. Sounds wasteful to me.

If you really want a bool, then cast the result like

signbit(x):bool
mppf commented 8 months ago

Current practice has the proc returning a bool result which means an extra operation would be needed during the routine.

It looks like it returns int(8) today?

Currently it is defined as

inline proc sgn(x : real(?w)): int(8) do
    return ((x > 0.0) : int(8) - (x < 0.0) : int(8)) : int(8);
mppf commented 8 months ago

Sorry, nevermind, I was mentally combining proc sgn and proc signbit.

proc signbit is currently unstable and defined as

inline proc signbit(x : real(64)): bool /* implemented by calling a C function */
damianmoz commented 8 months ago

To match the current definition, I would have to write

inline proc signbit(const x : real(?w))
{
    return ((x.transmute(uint(w))) >> (w - 1)) != 0;
}

which, unlike my post at the start, involves an additional comparison and looses me the number of bits in the original floating point number. And it sets me up for more work if I then wanted to retrieve the original bit as a uint, as I would need to do a further cast.

I think my original definition is simpler but it means a change to the current behaviour.

It should be pointed out that the current behavior is at odds with what occurs in C/C++ which always seemed strange to me. I have always wanted to know whether the negative bit field was a one or a zero.

I would also like to point out that whenever I have used this routine and been worried about when the argument was a NaN, I have always handled this case separately.

I think I got that right.