On Sun, 2024-04-07 15:22:42 +1000, Kalvis Duckmanton <kalvisd%gmail.com@localhost> 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
--
Attachment:
signature.asc
Description: PGP signature