Re: [PATCH] RISC-V: add riscv64 architecture support

[Rich Felker <dalias@libc.org>]

On Mon, Jun 03, 2019 at 04:46:56PM -0400, Drew DeVault wrote:
> Thanks for your patience, needed to wait until I returned from travel to
> take another look here.
> 
> On Sun May 26, 2019 at 12:31 AM Rich Felker wrote:
> > I don't understand why a_cas is defined here, and it's almost surely
> > wrong since it has no barriers whatsoever. If a_cas is left undefined,
> > it would be auto-defined in terms of a_ll and a_sc. But if we want to
> > follow the ISA docs which place restrictions on the positioning of
> > ll/sc and what can be done between them, I think we can't define a_ll
> > and a_sc, but have to define a_cas (with proper barriers) and let
> > everything else get defined in terms of it...
> 
> I don't grok the atomics code, to be honest. I'm not sure what the
> resolution is here.

If there's nobody who understands it available to work on it, I'm just
going to sub in an a_cas[_p]-only implementation for now, with full
barriers added before/after it, and we can discuss whether/how it can
be improved later.

> > This is probably gratuitous since there's no big endian ABI defined.
> > Or if the intent is to have a big endian ABI, that needs to be
> > reflected in reloc.h.
> 
> I don't know of any plans for a big-endian ABI. I've removed this for
> the future v2, and addressed simliar concerns throughout.

OK. If in doubt, leave it out. :-)

> > Is 2k min correct here? It's ABI so we should aim to get it right to
> > begin with.
> 
> The spec doesn't recommend a minimum stack size afaict, so this as
> reasonable as any other number to me.

OK. It fits current mcontext_t with plenty of spare room, so it should
be fine unless adding gigantic vector files like AVX512 is something
that's going to happen.

> > > +++ b/arch/riscv64/crt_arch.h
> > > -%<-
> > 
> > Is __global_pointer actually used in the hosted ABI?
> 
> Yes.

OK.

> > > +trycppif __riscv_float_abi_soft "$t" && SUBARCH=${SUBARCH}-sf
> > 
> > Is -sp also supposed to be supported?
> 
> Yes.

OK.

> > OK, that probably looks like a lot, but very little if anything from
> > it is blocking. I'm fine with taking patches or writing my own to fix
> > up things after the initial commit, just as long as we make sure the
> > initial commit doesn't have known ABI issues resolving whether big
> > endian is supposed to be supported, and whether single-precision fpu
> > is supposed to be supported, and what MINSIGSTKSZ should be, are the
> > main ones I see. I also think the atomics are probably broken enough
> > that it would be nice to get them fixed.
> 
> I've pushed a commit which addresses most of your feedback to the
> staging branch of https://github.com/riscv/riscv-musl.

Looks good!

> > I'm working on trying to get musl-cross-make in shape so that I can
> > test a build. At least binutils needs to be updated to a version with
> > riscv support, and in updating it I need to make sure I get a version
> > without the x86 regression or add a patch for it.. Hopefully I can
> > start testing in the next day or two.
> 
> I can also give you shell access to a RISC-V box if you would prefer
> to test with real hardware.

Thanks. I'll let you know if so, though I think qemu is fine for now.

Rich