RISC-V kernel-toolchain

[br]

Summary

Minimal set of changes required for "make kernel-toolchain" for RISC-V

[br]

Add context

[emaste]

Makefile.inc1

175

We should probably wrap this line (in a separate commit prior to this change), it's already very long.

etc/etc.riscv/ttys

2–5

Probably a candidate for https://wiki.freebsd.org/SrcDeDup

lib/csu/riscv/crt1.c

41–43

Remove this - see rS282551 / D2422. The other csu files should be done as well.

Remove historical GNUC test

The requirement is for a GCC-compatible compiler and not necessarily
GCC itself. However, we currently expect any compiler used for building    
the whole of FreeBSD to be GCC-compatible and many things will break if    
not; there's no longer a need to have an explicit test for this in rtld.

lib/csu/riscv/crti.S

6

These should probably be "Portions of this software were..." when there are two entries

share/mk/src.opts.mk

237–239

Perhaps we should list these on individual lines with a comment about why they fail, something like

BROKEN_OPTIONS+=PROFILE # missing mumble
BROKEN_OPTIONS+=RESCUE # crunchgen does not support RISC-V

[bdrewery]

Makefile.inc1

175–179

Please also update the TARGET_ARCHES list in share/mk/local.meta.sys.mk

lib/csu/riscv/Makefile

41–43

Please see the other csu directories. We're using FILES* now. Like this:

FILES= ${OBJS}
FILESMODE= ${LIBMODE}
FILESOWN= ${LIBOWN}
FILESGRP= ${LIBGRP}
FILESDIR= ${LIBDIR}

These FILES qualify as libraries for the purpose of LIBRARIES_ONLY.

.undef LIBRARIES_ONLY

[andrew]

etc/etc.riscv/ttys

45–52

You might want to update this, we have moved to use 3wire and onifconsole on these.

lib/csu/riscv/crt1.c

2

This file looks very similar to the arm64 version (it even has the _end hack). You should be using the existing license & adding yourself to it. The alternative is to redo it based on the amd64 version (which is similar).

lib/libc/riscv/Makefile.inc

4

arm?

lib/msun/riscv/Makefile.inc

4

What size is long double? On arm64 this is 113 as it's a 128-bit type.

lib/msun/riscv/fenv.h

2

This looks very similar to the arm64 fenv.h, the license may need to be checked.

[emaste]

lib/csu/riscv/crt1.c

42–44

Other csu files addressed in rS292000

[imp]

lib/msun/riscv/fenv.h

3

Sometimes I think we need to have a fp-ieee directory that has all this stuff in it that most ports these days just reference (or define one or two things to describe the flavor, then include).
Back when there were many types of floating point, the structure we had made sense. But once the Alpha adopted IEEE floating point, VAX format died in the wild and people only have done IEEE FP since then.

Of course, that may be too big an ask for someone porting to a new arch since we've not required it for others.

share/mk/bsd.cpu.mk

155

Before this you need something like
. elif ${MACHINE_CPUARCH} == "riscv"
_CPUFLAGS = -march=${CPUTYPE}
.endif

or maybe -mcpu depending on what gcc does.

I see references to riscv32 and riscv64 so maybe we need that distinction too? What did you do your port for?

163

this should have been added alphabetically.

287

You need to minimally have here

. elif ${MACHINE_ARCH} == "riscv"
MACHINE_CPU = riscv

here too. There's two places for each architecture we set it.

323

This isn't the compiler default?

share/mk/bsd.sys.mk

113

is this a riscv thing, or is it just a gcc 5.2.0 thing?

I'm not entirely sure this is the right place for this as well, but let's sort that bit out first.

share/mk/src.opts.mk

238–240

I'm curious why CXX is on the list too...

[emaste]

lib/msun/riscv/fenv.h

3

I think it's reasonable to move ahead with this as is but ensure it's documented on the SrcDeDup wiki page and come back and address them later on.

share/mk/bsd.cpu.mk

163

yep, we should move it in a separate commit prior to D4445 going in

[imp]

lib/msun/riscv/fenv.h

3

I'd settle for 'update SrcDeDup wiki' here.

share/mk/bsd.cpu.mk

163

Agreed. I was mostly grumpy since I thought I'd alphabetized these already. Turns out that mips is after powerpc, so my world was shattered again there. Let's actually move it after riscv goes in. It isn't important enough to force a rebase / merge cycle.

[emaste]

share/mk/bsd.cpu.mk

163

I've already forced a rebase cycle though :) I think Ruslan expected at least one iteration anyway.

My general approach has been to put it in the correct spot if it's already ordered or only one is out of place, or at the end otherwise.

[bdrewery]

Thanks, my findings look OK now. Approving just what I've commented on. I didn't look at other pieces.

[br]

share/mk/bsd.sys.mk

113

I think it is GCC 5.2.0

[andrew]

Makefile

350

Will universe work if the riscv toolchain is missing? We check for the external parts on arm64 & warn the user it was skipped.

lib/libc/riscv/Makefile.inc

2

The libc changes are incomplete, would it pay to drop them for now so we can review them as a complete unit.

share/mk/src.opts.mk

241

Where in the build is contents.ms from?

242

I'm guessing we just need to add EM_RISCV to usr.sbin/crunch/crunchide/exec_elf32.c.

[br]

Makefile

350

Will not work, let remove from UNIVERSE at this step

share/mk/src.opts.mk

241

Actually error is like that:
"libcxxrt.so: undefined reference to `_Unwind_Resume_or_Rethrow'"

242

yes, that works, thanks!

[emaste]

Makefile

350

Agreed.

lib/msun/riscv/fenv.h

4

I've added the files noted in this review to SrcDeDup

share/mk/src.opts.mk

241

This will be taken care of with a forthcoming llvm-libunwind import
You can see WIP here: https://github.com/emaste/freebsd/commits/arm64-llvm-libunwind

242

It turns out I had that change sitting in an old work branch but did not have a way to test -- I've committed it now.

[andrew]

lib/msun/riscv/fenv.h

56

These are in the wrong order

63–66

These should be the values as applied directly to the register, e.g.

#define FE_UPWARD (3 << 5)

84–86

This looks wrong, bits 31:8 should be zero.

[imp]

share/mk/bsd.cpu.mk

154

You can't have this as _CPUFLAGS. It is an ABI affecting flag, so must be added directly to CFLAGS.
This line should instead read something like

_CPUCFLAGS = -march=$CPUTYPE

since that generally doesn't affect the ABI, just the set of instructions used and their relative costs for the optimizer. -msoft-float, however, changes calling conventions (at least on ARM it does), so must either be the compiler default (preferred) or always added to the command line (second best). There's a spot further down in the file that does this.

share/mk/bsd.endian.mk

7

I'd be tempted to use MACHINE_CPUARCH here, since all members of the family are little endian, right?

share/mk/bsd.sys.mk

113

If so, it should be based on compiler version only, not on cpu arch.

[br]

fenv.h changed

[andrew]

lib/msun/riscv/fenv.h

64

This should be (0x03 << 5)

66

This should be (0x01 << 5)

share/mk/bsd.cpu.mk

154

I would expect we would move to the hard-float ABI when the kernel supports this, however we are currently missing the kernel bits.

[br]

mk fixed

[br]

fenv.h fixed