[PATCH 3/3] or1k: gcc: initial support for openrisc
Stafford Horne
shorne@gmail.com
Tue Aug 28 13:04:00 GMT 2018
Hi Joseph,
On Mon, Aug 27, 2018 at 05:24:51PM +0000, Joseph Myers wrote:
> On Mon, 27 Aug 2018, Stafford Horne wrote:
>
> > gcc/config/or1k/elf.opt | 33 +
>
> > gcc/config/or1k/or1k.opt | 41 +
>
> Command-line options need documenting in invoke.texi. This patch is
> missing documentation updates. Please see sourcebuild.texi, section "Back
> End", for a list of the various places that need updating for a new target
> architecture, including the various places in the documentation that need
> updating if the architecture has any instances of the feature documented
> there.
Sure I will document these. I was thinking about what needed to be documented,
thanks for pointing this out.
> > +#define TARGET_FLOAT_FORMAT IEEE_FLOAT_FORMAT
>
> This target macro was obsoleted in 2008. You shouldn't be defining it
> here.
>
> Whereas TARGET_FLOAT_FORMAT is missing a poisoning in system.h, such as
> should be done for obsolete target macros to make it easy to spot when an
> out-of-tree port is defining them, the obsolete NO_IMPLICIT_EXTERN_C,
> which you're defining in or1k/linux.h, is properly poisoned in system.h,
> so I'd have expected the build for that target to fail.
I will fix the TARGET_FLOAT_FORMAT. However, I am not sure why
NO_IMPLICIT_EXTERN_C is not causing a compile failure. This definitely has been
working and tested with musl libc.
I will let you know what I find.
>
> You're using /lib/ld.so.1 as GLIBC_DYNAMIC_LINKER. Preferred glibc
> practice for new ports is to have an architecture-specific, ABI-specific
> dynamic linker name, not shared with any other ABI or architecture, to
> support distributions using multi-arch directory arrangements.
Sure, I will do that. We have an existing glibc port which I haven't touched
before, I will see what it expects and set it up that way.
> Why the __BIG_ENDIAN__ built-in macro? Since we have the
> architecture-independent __BYTE_ORDER__ macro, new architectures shouldn't
> generally define their own macros related to byte-order (especially for an
> architecture that only supports one endianness!) unless it's trying to
> implement some compiler-independent API for that architecture that says
> such macros are defined.
I think this just comes from me copying an example from m32r. I don't think its
needed and I will remove it.
Thank you for the review, I would not have spotted these. If anything is
missing in the gccint docs which might make it easier for future porters to not
make these mistakes I will try to add it too.
-Stafford
More information about the Gcc-patches
mailing list