Re: [PATCH] add initial support for J2 core to sh target

[Oleg Endo <oleg.endo@t-online.de>]

Hi Rich,

On 01 Sep 2015, at 02:49, Rich Felker <dalias@libc.org> wrote:

> The J2 Core is an open hardware cpu implementing the SH-2 instruction
> set, with the addition of barrel shift instructions and an atomic
> compare-and-swap instruction. This patch adds a cpu model option -mj2
> to the sh target. Presently all it does is enable use of the barrel
> shift instructions (and turns off assembler checking of the ISA via
> --isa=any) but I will eventually add support for the new CAS
> instruction as a new -matomic-model for use by the __sync and __atomic
> builtins.

It seems that this J2 atomic instruction(s ?) is not available to the public.  I've skimmed through the currently available J2 hardware sources, but couldn't find anything about it.  So it's just speculation, but probably you'll require a copyright assignment for the atomics parts.

> I've used the the name "J2" and "-mj2" rather than treating it as a
> submodel variant of "sh2" ("SH2J" and "-m2j") because the official
> name of this cpu model is "J2", with the intent of not misrepresenting
> it as a Renesas product.

But then, why do you add "shj2" in config.gcc?
I guess having an SH variant which doesn't match sh*-*-* would be odd.  I'd rather name it "SHJ2" everywhere (except the -m options which don't have the "sh" prefix).  Technically it really is just another SH variant so I'd suggest to treat it like that.  Might be simpler and less confusing for other things than GCC, too.

Also you might want to add the new option to that block in config.gcc

	echo "Unknown CPU used in --with-cpu=$with_cpu, known values:"  1>&2
	echo "m1 m2 m2e m3 m3e m4 m4-single m4-single-only m4-nofpu" 1>&2
	echo "m4a m4a-single m4a-single-only m4a-nofpu m4al" 1>&2
	echo "m2a m2a-single m2a-single-only m2a-nofpu" 1>&2
	exit 1
	;;

> The --isa=any passed to the assembler probably needs to be fixed
> before this patch is ready for upstream (although I'd really just
> prefer _always_ passing --isa=any to the assembler, since the current
> behavior breaks runtime-switching implementations, which I need).

I've added the --isa thing because LD also checks (or at least tries) compatibility of modules being linked together.  Also, it makes it easier to detect wrong/unexpected instructions in the output, which can happen sometimes by a bug in GCC or with inline-asm.  If you have other suggestions, please share.

> One
> complication is that passing --isa=j2 will fail with old binutils; I'm
> not sure if this should be detected and handled or if we should just
> require up-to-date binutils to use -mj2. In any case, I don't yet have
> an assembler-side patch to add the --isa level.

I'd say requiring new binutils is acceptable.  But that'd require being able to get a new binutils to begin with :)

Binutils will most likely require patches for the J2 atomic insn.  Thus maybe it's better to start with binutils first.

> I know this isn't ready for upstream yet but I'd appreciate feedback
> on the patch so far and anything that I need to change in order for it
> to be acceptable.

A changelog entry and documentation (new -m options etc) would be good.

Cheers,
Oleg