Re: [PATCH v2 3/4] target/s390x: Implement DIVIDE TO INTEGER

[Ilya Leoshkevich <iii@linux.ibm.com>]

On 2026-01-28 21:38, Richard Henderson wrote:
> On 1/29/26 00:19, Ilya Leoshkevich wrote:
>> With some testcases I hit this condition in 
>> parts_round_to_int_normal():
>> 
>>      if (a->exp >= frac_size) {
>>          /* All integral */
>>          return false;
>>      }
>> 
>> which makes it a no-op.
>> 
>> I think the code assumes that FloatParts have just been unpacked and 
>> all low fraction bits are zero, which is not the case for quotient 
>> here.
>> 
>>>> +        float_exception_flags = saved_flags;
>>>> +        parts_s390_precision_round_normal(&n128, fmt);
>>> 
>>> parts_round_to_int_normal already takes a rounding mode and 
>>> frac_size.
>>> It also returns whether or not the rounding was exact.
>>> This appears to be trying to reinvent the wheel.
>> 
>> 
>> Apparently I use this to paper over the above issue, which is not 
>> great.
>> 
>> I guess improving parts_round_to_int() to work with non-zero low 
>> fraction bits would be better.
>> 
>> What do you think?
> 
> Yes indeed.  I think (a->exp >= N) should be sufficient?

Seems like this does not work for really large quotients, because we 
still need to trim the fraction bits.
So I'm currently evaluating the following, which looks promising so far:

--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -1118,11 +1118,6 @@ static bool 
partsN(round_to_int_normal)(FloatPartsN *a, FloatRoundMode rmode,
          return true;
      }

-    if (a->exp >= frac_size) {
-        /* All integral */
-        return false;
-    }
-
      if (N > 64 && a->exp < N - 64) {
          /*
           * Rounding is not in the low word -- shift lsb to bit 2,
@@ -1133,7 +1128,7 @@ static bool 
partsN(round_to_int_normal)(FloatPartsN *a, FloatRoundMode rmode,
          frac_lsb = 1 << 2;
      } else {
          shift_adj = 0;
-        frac_lsb = DECOMPOSED_IMPLICIT_BIT >> (a->exp & 63);
+        frac_lsb = DECOMPOSED_IMPLICIT_BIT >> MIN(a->exp, frac_size);
      }

> r~