The vector components were mistakenly using () instead of {}, which caused

all but the last vector component to be dropped on the floor.

Signed-off-by: Aaron Watry <awatry@gmail.com>

The vector components were mistakenly using () instead of {}, which caused

all but the last vector component to be dropped on the floor.

Signed-off-by: Aaron Watry <awatry@gmail.com>

Hi Aaron,

This change looks good to me. However, I’m still not totally convinced this patched version does the right thing. When I read this:

it seems that in the scalar float case either 0 or 1 should be returned and in the vector case a vector filled with 0s and -1s (minus ones).

Jeroen

And that's what it should be doing.

For scalar signbit, we return:

return __builtin_signbitf(x);

For vector values of float2 for example, we return

(int2)( (int2){__builtin_signbitf(x.s0), __builtin_signbitf(x.s1)} != (int2) 0)

__builtin_signbitf(float) returns either 0 or 1 depending on

false/true. We need to convert that value from 1 to -1 for vector

calls.

The '!= 0' comparison takes care of returning -1 for us for vector

calls, and we skip that comparison for scalar calls and just return

__builtin_signbitf's result directly.

In the bitcode from the previous email,

%7 = insertelement <3 x i32> undef, i32 %.lobit.i.i, i32 0

%8 = bitcast float %3 to i32

%.lobit.i3.i = lshr i32 %8, 31

%9 = insertelement <3 x i32> %7, i32 %.lobit.i3.i, i32 1

%10 = bitcast float %5 to i32

%.lobit.i2.i = lshr i32 %10, 31

%11 = insertelement <3 x i32> %9, i32 %.lobit.i2.i, i32 2

%12 = icmp ne <3 x i32> %11, zeroinitializer

%13 = sext <3 x i1> %12 to <3 x i32>

%7, %9, and %11 build a <3 x i32> vector

%12 compares the vector to 0

%13 sign extends the result from <3 x i1> to <3 x i32>

Unless I'm very mistaken, that's exactly what we want here.

--Aaron

Hi,

You’re correct. Mea culpa, I missed the fact that != returns -1 for vectors.

So, looks good to me!

Jeroen