Re: RISC-V glibc port, v4

[Palmer Dabbelt <palmer at dabbelt dot com>]

On Mon, 15 Jan 2018 08:13:52 PST (-0800), joseph@codesourcery.com wrote:
> On Sat, 13 Jan 2018, Palmer Dabbelt wrote:
>
> > It's been a bit more than two weeks since the last patch set, and I think we're
> > starting to get something that's in reasonable shape.  Thanks to Darius for
> > putting in a lot of time into the v4.  We've also gotten the test suite in a
> > bit better shape: we're down to 200 failures when running natively on Linux,
> > and all the test suite failures when running in user mode emulate appear to be
> > environment issues.  As far as I know the only remaining issue in our port is
> > to triage the test suite failures, most of which we're hoping are just
> > deficiencies in our current environment.  Of course, now that I've said that
> > I'm sure we'll find a bunch of bugs all over the place :)
>
> Although no more than about 20 architecture-specific failures (for each
> configuration tested) should be expected for a port in reasonable shape
> for inclusion, even before that point it makes sense to provide the list
> of failures, as some failures may point reviewers to specific issues with
> the port - details of test failures are necessary information for a full
> review of a port.  (Even better, also make the "make check" output and the
> .out files for failed tests available somewhere.)
>
> > Thanks to everyone who has helped review and contributed to our port.  Like
> > usual I believe we've taken everything into account (and hopefully Darius' hit
> > rate is a bit better than mine), but if we've missed anything feel free to ping
> > it and we'll take another look.
>
> * This version is still missing leading __ on the parameter names for
> __riscv_flush_icache in sys/cachectl.h.

Sorry about that, this will be fixed for the v5.

> * Of the pieces of multi-ABI support I mentioned in
> <https://sourceware.org/ml/libc-alpha/2018-01/msg00008.html>, you're
> missing dl-cache.h and an ldd_rewrite_script setting.  (To test all this
> you need to set up a system with libraries for multiple ABIs installed in
> the appropriate directories, run ldconfig and make sure that the libraries
> for the different ABIs are properly listed in the cache with the right
> flags set on them - and that ldd installed with glibc for one ABI works
> properly on binaries for both that and the other ABI.  The glibc testsuite
> probably doesn't cover any of this.)

OK, thanks.  I think I've got this sort of figured out, I'll try to 

> * You're missing sysdeps/riscv/nofpu/Implies pointing to ieee754/soft-fp
> (or more nofpu directories than that, if a single such Implies file
> doesn't suffice to get ieee754/soft-fp before the other ieee754
> directories in the sysdeps directory ordering for soft-float
> configurations).
>
> * As I said in
> <https://sourceware.org/ml/libc-alpha/2017-06/msg00645.html>, you should
> not have a riscv/soft-fp directory.  Previous versions got this right,
> this one appears to have gone backwards.

Looks like we got some signals crossed here, I think I've done it correctly 
this time:

   diff --git a/sysdeps/riscv/nofpu/Implies b/sysdeps/riscv/nofpu/Implies
   new file mode 100644
   index 000000000000..abcbadb25f22
   --- /dev/null
   +++ b/sysdeps/riscv/nofpu/Implies
   @@ -0,0 +1 @@
   +ieee754/soft-fp
   diff --git a/sysdeps/riscv/preconfigure b/sysdeps/riscv/preconfigure
   index a486a271031e..a69b11e59738 100644
   --- a/sysdeps/riscv/preconfigure
   +++ b/sysdeps/riscv/preconfigure
   @@ -20,7 +20,7 @@ riscv*)
    	with_fp_cond=1
        	;;
        "")
   -    	float_machine=
   +	float_machine=nofpu
    	with_fp_cond=0
        	;;
        *)

I'll play around a bit and make sure the soft-float stuff lines up.

> > * Copyright year has been updated to 2018, and "contributed by" lines have been
> >   removed.
>
> But you have one in sysdeps/unix/sysv/linux/riscv/readelflib.c (which is
> also missing an explanatory comment before the copyright notice).

Sorry about that, that one snuck in after the change.  I've fixed it.