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

[Rich Felker <dalias@libc.org>]

On Tue, Sep 01, 2015 at 10:45:10PM +0900, Oleg Endo wrote:
> 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.

It's still under development and I'm not closely following the
hardware side of things. I'll hold off on submitting the atomic
support until it's in the public release and tested. My hope was to
get the basic support upstream first (having -mj2 is very helpful for
building the kernel since it makes the libgcc issue we were dealing
with go away) and add the atomic part later.

From the copyright side, though, IMO it's not a matter of whether the
feature is public but the non-triviality of the patch that would make
it require an assignment. My work on this is under contract with SE
Instruments and our arrangement is such that they're responsible for
copyright/assignment on the work I do on FSF projects. AFAIK they
don't yet have any assignment paperwork on file for this so I'll need
some guidance from whoever handles that for GCC.

> > 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?

Because the code it's added to just did s/m/sh/ as part of its text
processing. ;-) It's slightly ugly shell script but I'm just working
with what's there.

> 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
> 	;;

Yes, I agree it could be odd from the GCC side. That's actually why I
omitted tuples for now and just wanted to use -mj2 or --with-cpu: I
was concerned configure scripts would blow up on seeing a cpu family
name they don't know.

If you want to just follow the existing naming pattern for tuples, I
think "sh2j" might make more sense than "shj2". It would both be
easier to match (there's probably a lot of software that accepts
sh[:digit:] but not sh[:alnum:]) and it's sufficiently different from
the actual name of the cpu so as not to give the impression that "J2"
is short for "SH J2" or something.

> > 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.

I'm not sure what the best way to achieve multiple goals is, but the
current behavior makes it so you need --isa=any (and a final binary
with weird ABI tag) to have a binary that supports atomic operations
on any SH model. musl libc already has such support (except the new J2
CAS instruction) and I would like to eventually provide a libatomic
approach for GCC too so that it's possible to use __sync/C11 atomics
and have the binary be safe to run on any model that supports the
baseline ISA & ABI you built for (e.g. all >=SH2 if you used -m2).

> > 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 have a patch for that part, just not expanding the
already-very-complex SH "family-tree" of instruction support. However
it's likely that encoding details will change (the draft encoding
overlaps with something used by SH2A IIRC, and the intent was not to
have such overlap) so I'm holding off on submitting it until the
hardware side works out this issue. This draft patch and all the
others I currently need for a working toolchain are available here:

https://github.com/richfelker/musl-cross-make/tree/master/patches

> > 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.

OK.

Rich