David: Ping.
On Thu, 2024-01-25 at 16:04 -0500, Antoni Boucher wrote:
> Thanks for the review!
>
> On Wed, 2024-01-24 at 13:10 -0500, David Malcolm wrote:
> > On Thu, 2024-01-11 at 18:42 -0500, Antoni Boucher wrote:
> > > Hi.
> > > This patch fixes the bug 113343.
> > > I'm wondering if there's a better solution than using mpfr.
> > > The only other solution I found is real_from_string, but that
> > > seems
> > > overkill to convert the number to a string.
> > > I could not find a better way to create a real value from a host
> > > double.
> >
> > I took a look, and I don't see a better way; it seems weird to go
> > through a string stage. Ideally there would be a
> > real_from_host_double, but I don't see one.
> >
> > Is there a cross-platform way to directly access the representation
> > of
> > a host double?
>
> I have no idea.
>
> >
> > > If there's no solution, do we lose some precision by using mpfr?
> > > Running Rust's core library tests, there was a difference of one
> > > decimal, so I'm wondering if there's some lost precision, or if
> > > it's
> > > just because those tests don't work on m68k which was my test
> > > target.
> >
> > Sorry, can you clarify what you mean by "a difference of one
> > decimal"
> > above?
>
> Let's say the Rust core tests expected the value "1.23456789", it
> instead got the value "1.2345678" (e.g. without the last decimal).
> Not sure if this is expected.
> Everything works fine for x86-64; this just happened for m68k which
> is
> not well supported for now in Rust, so that might just be that the
> test
> doesn't work on this platform.
>
> >
> > > Also, I'm not sure how to write a test this fix. Any ideas?
> >
> > I think we don't need cross-compilation-specific tests, we should
> > just
> > use and/or extend the existing coverage for
> > gcc_jit_context_new_rvalue_from_double e.g. in test-constants.c and
> > test-types.c
> >
> > We probably should have test coverage for "awkward" values; we
> > already
> > have coverage for DBL_MIN and DBL_MAX, but we don't yet have test
> > coverage for:
> > * quiet/signaling NaN
> > * +ve/-ve inf
> > * -ve zero
>
> Is this something you would want for this patch?
>
> >
> > Thanks
> > Dave
> >
>
