Re: VAX: Another small GAS bug?

On Sun, 2024-04-07 15:22:42 +1000, Kalvis Duckmanton <[email protected]> wrote:
> On 7/4/24 05:01, Jan-Benedict Glaw wrote:
> > I haven't even _looked_ at the code, so I don't know what it's
> > actually doing. I don't know the FP representation out of my head, but
> > if an all-zero value is interpreted as 0.0, a CLRx would probably be
> > enough? Still, I'd like to see that bug fixed (and maybe add a
> > testcase for it to upstream Binutils.)
> > 
> >    Unfortunately, I guess I'll only have time for this in a month or
> > the like, maybe you, or Kalvin, or Anders, or Martin, or Maciej, or
> > Mouse, or ... may want to have a look?  Esp, since it happened in a
> > regular CI build, but I was unable (in a few-minutes attempt) to
> > reproduce it. :-/  I'd like to make sure there isn't much more hidden
> > behind this issue.
> 
> So I had a look today.  I don't believe GAS is at fault; it has assembled an
> F-floating zero (so 32 bits wide) ... into the lower part of a buffer big
> enough to hold a quadword (64 bits) or a D-floating value (again 64 bits). 
> The other 32 bits in that buffer, which are mantissa bits in the D-floating
> type, are not initialised.  As rin points out, VAX looks only at the sign
> and exponent fields to determine whether a value is a zero.
> 
> The easiest fix seems to be to change the constants from $0f0.0 to $0d0.0. 
> If I do that and reassemble unimpl_emul.S (using native GAS), I get
> 
>  478:   c8 02 ad 40     bisl2 $0x2,0x40(fp)
>  47c:   d1 58 38        cmpl r8,$0x38
>  47f:   19 0f           blss 490 <emodd+0x1e8>
>  481:   7d 8f 00 00     movq $0x0000000000000000,0x20(ap)
>  485:   00 00 00 00
>  489:   00 00 ac 20
>  48d:   17 af 78        jmp 508 <emodd+0x260>
> 
> as expected.  I've shown only one example but the other 2 are the same - no
> unexpectedly set bits.

I added some debugging code to flonum_gen2vax() last night and
realized we go this route:

266   if (return_value != 0)
267     make_invalid_floating_point_number (words);
268 
269   else
270     {
271       if (f->low > f->leader)
272         /* 0.0e0 seen.  */
273         memset (words, '\0', sizeof (LITTLENUM_TYPE) * precision);

...through the memset(). With movQ/movD, that's too small
(LITTLENUM_TYPE being 16 bits and precision = 2).

> diff --git a/sys/arch/vax/vax/unimpl_emul.S b/sys/arch/vax/vax/unimpl_emul.S
> index c3b7b83baa3a..a546ceda3e2b 100644
> --- a/sys/arch/vax/vax/unimpl_emul.S
> +++ b/sys/arch/vax/vax/unimpl_emul.S
> @@ -683,7 +683,7 @@ emodd:      bsbw    touser
>          * there aren't any bits left for the fraction.  Therefore we're
>          * done here;  TMPFRAC1 is equal to TMPFRACTGT and TMPFRAC2 is 0.
>          */
> -       movq $0f0.0, TMPFRAC2
> +       movq $0d0.0, TMPFRAC2
>         jmp 9f          /* we're done, move on */
>  1:
>         /*
> @@ -727,7 +727,7 @@ emodd:      bsbw    touser
>          * We are less than 1.0; TMPFRAC1 should be 0, and TMPFRAC2 should
>          * be equal to TMPFRACTGT.
>          */
> -       movd $0f0.0, TMPFRAC1
> +       movd $0d0.0, TMPFRAC1
>         movd TMPFRACTGT, TMPFRAC2
>  9:
>         /*
> @@ -763,7 +763,7 @@ zeroexit:
>         bsbw getaddr_byte
>         movl $0x0, (%r0)
>         bsbw getaddr_byte
> -       movd $0f0, (%r0)
> +       movd $0d0, (%r0)
>         brw goback
> 
> 
That is IMHO the exact correct thing to do. Who can commit that?
Anders? Martin?

MfG, JBG

--

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQQlDTvPcScNjKREqWEdvV51g5nhuwUCZhI3XwAKCRAdvV51g5nh
u/agAJ9FfzL21Pj7y3LsIrroPjWXUmpq1ACeI/6r1oK8+7aXIqzDh+BpatQbduo=
=jPnj
-----END PGP SIGNATURE-----