On 28.03.2011, at 19:55, Peter Maydell wrote:
> On 28 March 2011 18:23, Alexander Graf <ag...@suse.de> wrote:
>> On 03/24/2011 06:29 PM, Peter Maydell wrote:
>>>> +/* condition codes for binary FP ops */
>>>> +static uint32_t set_cc_f32(float32 v1, float32 v2)
>>>> +{
>>>> + if (float32_is_any_nan(v1) || float32_is_any_nan(v2)) {
>>>> + return 3;
>>>> + } else if (float32_eq(v1, v2,&env->fpu_status)) {
>>>> + return 0;
>>>> + } else if (float32_lt(v1, v2,&env->fpu_status)) {
>>>> + return 1;
>>>> + } else {
>>>> + return 2;
>>>> + }
>>>> +}
>>>
>>> Can you not use float32_compare_quiet() (returns a value
>>> telling you if it's less/equal/greater/unordered)?
>>> If not, needs a comment saying why you need to do it the hard way.
>>
>> I just checked the macros there and it looks like float32_compare_quiet
>> returns eq when both numbers are NaN.
>
> Hmm?
>
> if (( ( extractFloat ## s ## Exp( a ) == nan_exp ) && \
> extractFloat ## s ## Frac( a ) ) || \
> ( ( extractFloat ## s ## Exp( b ) == nan_exp ) && \
> extractFloat ## s ## Frac( b ) )) { \
> if (!is_quiet || \
> float ## s ## _is_signaling_nan( a ) || \
> float ## s ## _is_signaling_nan( b ) ) { \
> float_raise( float_flag_invalid STATUS_VAR); \
> } \
> return float_relation_unordered; \
> } \
>
> If A is a NaN (ie its exponent is nan_exp and the frac bits aren't zero)
> or B is a NaN then we return float_relation_unordered.
>
>> We would still have to convert from
>> the return value from that over to a CC value. I honestly don't see any
>> benefit - the code doesn't become cleaner or smaller.
>
> So you get
> static uint32_t set_cc_f32(float32 v1, float32 v2)
> {
> switch (float32_compare_quiet(v1, v2, &env->fpu_status)) {
> case float_relation_unordered:
> return 3;
> case float_relation_equal:
> return 0;
> case float_relation_less:
> return 1;
> case float_relation_greater:
> return 2;
> case float_relation_unordered:
> return 3;
> }
> }
>
> (and you probably want to put the conversion switch into a function
> since you'll be using it several times.)
>
> Which I think is pretty straightforward, cleaner because we only
> call one softfloat function rather than several, and should be
> faster too (we get to avoid repeating a pile of tedious bit manipulation
> in the eq and lt functions).
Alrighty, changed the code :).
>
>>>> +/* load 128-bit FP zero */
>>>> +void HELPER(lzxr)(uint32_t f1)
>>>> +{
>>>> + CPU_QuadU x;
>>>> + x.q = float64_to_float128(float64_zero,&env->fpu_status);
>>>
>>> Yuck. Just define a float128_zero if we need one.
>>
>> Good point. Mind to do so? I find myself struggling with the code there.
>
> We could just follow the pattern of float128_default_nan_{low,high}
> in softfloat.h:
>
> #define float128_zero_low LIT64(0)
> #define float128_zero_high LIT64(0)
>
> then your function has:
> x.q.high = float128_zero_high;
> x.q.low = float128_zero_low;
>
> Or we could do something with an expression that returns a
> struct type; that would be cleaner. I think the default nan
> code is assuming it might have to be compiled with something
> other than gcc. However I forget the C syntax and have to go
> home now :-)
I'll just leave it to you for a follow-up patch :). The less I have to touch in
softfpu code, the better :)
Alex
