This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/3] or1k: gcc: initial support for openrisc

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.

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]