This is the mail archive of the gcc-patches@gcc.gnu.org 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: [09/25] Specs cleanup: CRIS


> Date: Fri, 14 Jan 2011 00:08:24 +0000 (UTC)
> From: "Joseph S. Myers" <joseph@codesourcery.com>

> This patch fixes some issues with the specs for CRIS targets.
> 
> * Various specs checked for a -cpu option that should have been
>   -mcpu.  (These could probably be simplified further by making -mcpu=
>   a .opt alias for -march=, after which specs wouldn't need to check
>   for it at all, but I'm not doing that in this patch series.)

Right, a typo introduced with r130970, as can be seen in the
error text.  For proper context, I should mention that this
predates the Alias parts of the .opt machinery, thanks for the hint.

> * cris/linux.h had specs passing -rpath-link options in some cases
>   (-B, or not -nostdlib).  This is not needed for any properly
>   configured cross toolchain (for GNU/Linux targets, that means using
>   a sysroot).

A sysroot configuration is not (should not be) mandatory for a
working cross-toolchain; no particular configure options at all
should be needed besides the --target option.  I understand
the confusion seeing that "down" in a port though.  I traced
this to
<http://gcc.gnu.org/ml/gcc-patches/2009-03/msg01004.html>.

The -rpath-link's are mentioned in comments, both in linux.h and
(stale placement) cris.h so for the patch to be complete, those
should be edited too.  While IMO this cleanup is not as
important as testing to work out-of-the box (without extra
.exp's adding options all over), having the same behavior over
all toolchains is more important, so ok-by-a-hair, if you edit
the comments to remove the -rpath-link.  If nothing else, it'll
remind me to (try and) fix this TRT the next time, at which time
I may add these back with better comments if all else fails. ;-)
(Either way, at least some parts should be conditionalized in
some way and not included in all builds, polluting the spec-space.)

>  And if it were appropriate, it should be done for all
>   GNU/Linux targets since there is nothing meaningfully CRIS-specific
>   about it.

Definitely.  I think there should be a generic GNU/Linux
"%{B*:-rpath-link %* }".

> * cris/link.h had a spec %{shlib:-Bdynamic} - that is, defining a
>   -shlib option to mean that -Bdynamic is passed to the linker.
>   Again, this is not logically CRIS-specific; if there were a need for
>   such an option rather than -Wl,-Bdynamic (and -Bstatic and -Bdynamic
>   are generally passed in pairs) it should be for all GNU/Linux
>   targets.  This undocumented option is removed in this patch.

The use and need for that legacy option has long gone so I guess
ok, but in general I don't see anything wrong with undocumented
legacy options.

> OK to commit?

With the -rpath-link parts of comments removed too.

brgds, H-P


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