[Bug target/50694] New: SH Target: SH2A little endian does not actually work

[Bug target/50694] New: SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

             Bug #: 50694
           Summary: SH Target: SH2A little endian does not actually work
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: oleg.endo@t-online.de


While testing a patch I have noticed some failures for SH2A and -ml.
The problem is that GCC happily generates SH2A code in little endian, but
when the output is assembled with GAS and it contains a "movi20" instruction,
GAS will abort at some "is big endian?" check. 

Maybe for SH2A the -ml option should either output a warning and do big endian,
or just abort with an error message?
As far as I know, the SH2A does big endian only anyway...

Cheers,
Oleg

[Bug target/50694] SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #1 from Oleg Endo <oleg.endo@t-online.de> 2011-10-13 19:54:31 UTC ---
As it turns out, this is already handled by the line

#define DRIVER_SELF_SPECS "%{m2a:%{ml:%eSH2a does not support little-endian}}"

in sh.h.

However, it doesn't catch -m2a-nofpu, -m2a-single, -m2a-single-only options.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-10-16
     Ever Confirmed|0                           |1

--- Comment #2 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-16 23:28:48 UTC ---
(In reply to comment #1)
Ah.  One liner

-#define DRIVER_SELF_SPECS "%{m2a:%{ml:%eSH2a does not support little-endian}}"
+#define DRIVER_SELF_SPECS "%{m2a*:%{ml:%eSH2a does not support
little-endian}}"

should work.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #3 from Oleg Endo <oleg.endo@t-online.de> 2011-10-18 21:33:06 UTC ---
(In reply to comment #2)
> Ah.  One liner
> 
> -#define DRIVER_SELF_SPECS "%{m2a:%{ml:%eSH2a does not support little-endian}}"
> +#define DRIVER_SELF_SPECS "%{m2a*:%{ml:%eSH2a does not support
> little-endian}}"
> 
> should work.

Yep, does work. 
However, I've noticed that it will stop working when the compiler's default
SH CPU and endian configuration is not -m1 -mb.

The code below instead gets the job done but I'm not sure whether it could
be written in a better way.
I've also noticed, that SH1 also supports big endian only, and -m1 -ml is
explicitly excluded in the multilib config in t-sh.

#if TARGET_ENDIAN_DEFAULT == MASK_BIG_ENDIAN
#define IS_LITTLE_ENDIAN_OPTION "%{ml:"
#else
#define IS_LITTLE_ENDIAN_OPTION "%{!mb:"
#endif

#if TARGET_CPU_DEFAULT == SELECT_SH1
#define UNSUPPORTED_SH1 IS_LITTLE_ENDIAN_OPTION \
"%{m1|!m2*:%{!m3*:%{!m4*:%{!m5*:%eSH1 does not support little-endian}}}}}"
#else
#define UNSUPPORTED_SH1 IS_LITTLE_ENDIAN_OPTION \
"%{m1:%eSH1 does not support little-endian}}"
#endif

#if TARGET_CPU_DEFAULT & MASK_HARD_SH2A
#define UNSUPPORTED_SH2A IS_LITTLE_ENDIAN_OPTION \
"%{m2a*|!m1:%{!m2*:%{!m3*:%{!m4*:{!m5*:%eSH2a does not support
little-endian}}}}}}"
#else
#define UNSUPPORTED_SH2A IS_LITTLE_ENDIAN_OPTION \
"%{m2a*:%eSH2a does not support little-endian}}"
#endif

#define DRIVER_SELF_SPECS UNSUPPORTED_SH1, UNSUPPORTED_SH2A



Actually, SH2E also supports big endian only as per public specification.
But I don't think that it would cause any trouble for anyone.  Of course it
could be excluded as well. :)

[Bug target/50694] SH Target: SH2A little endian does not actually work

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-18 22:24:32 UTC ---
(In reply to comment #3)
There are no real uses of SH1/SH2/SH2E/SH3E cores anymore, I think.
I agree that taking care of -m2e is not worth.  Perhaps same for
-m1.  Anyway, your change looks plausible to me.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #5 from Oleg Endo <oleg.endo@t-online.de> 2011-10-18 22:37:13 UTC ---
(In reply to comment #4)
> There are no real uses of SH1/SH2/SH2E/SH3E cores anymore, I think.

True. But the SH7020 (SH1) is still listed on digikey for an amazing
collector's price ;)

> I agree that taking care of -m2e is not worth.  Perhaps same for
> -m1.  Anyway, your change looks plausible to me.

I'll send in a patch with a couple of other cosmetic changes later, OK?

[Bug target/50694] SH Target: SH2A little endian does not actually work

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #6 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-18 22:50:19 UTC ---
(In reply to comment #5)
> I'll send in a patch with a couple of other cosmetic changes later, OK?

Please go for it.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #7 from Oleg Endo <oleg.endo@t-online.de> 2011-10-20 18:44:32 UTC ---
(In reply to comment #6)
> (In reply to comment #5)
> > I'll send in a patch with a couple of other cosmetic changes later, OK?
> 
> Please go for it.

..or maybe just leave it as it is :T
The change I was suggesting opens up another problem with multilib and endian
config / selection.
I think instead of adding / implementing the endian restrictions it would be
more useful to expand little endian support in binutils and drop the endian
restrictions in gcc altogether once binutils fully support it.

What do you think? Would that make more sense in the end?

[Bug target/50694] SH Target: SH2A little endian does not actually work

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #8 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-20 22:40:27 UTC ---
(In reply to comment #7)

This problem doesn't require the theoretical/mathematical
completeness.  There are many inappropriate combinations
of options which don't get any warning when running compiler
and configurations.  The important thing is to warn very
confusing ones from the user's point of view.  So your patch
in #6 or even one liner in #2 would be OK and enough for
this PR, I think.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[oleg.endo@t-online.de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #9 from Oleg Endo <oleg.endo@t-online.de> 2011-10-30 12:45:30 UTC ---
(In reply to comment #8)
> (In reply to comment #7)
> 
> This problem doesn't require the theoretical/mathematical
> completeness.  There are many inappropriate combinations
> of options which don't get any warning when running compiler
> and configurations.  The important thing is to warn very
> confusing ones from the user's point of view.  So your patch
> in #6 or even one liner in #2 would be OK and enough for
> this PR, I think.

OK.  I think the SH1 check should be left out completely in this case.
If it is included, the default configuration (-m1 -mb) will fail to build
e.g. newlib.  I'd propose catching the SH2A case only, considering the user's
default -mb / -ml configuration.  If somebody configures the default to be -m2a
-ml then it will fail to build, but I guess only very few people will try to do
that anyway.

[Bug target/50694] SH Target: SH2A little endian does not actually work

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

--- Comment #10 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-11-13 23:00:15 UTC ---
Author: kkojima
Date: Sun Nov 13 23:00:10 2011
New Revision: 181340

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181340
Log:
    PR target/50694
    * config/sh/sh.h (IS_LITTLE_ENDIAN_OPTION, UNSUPPORTED_SH2A):
    New macros.
    (DRIVER_SELF_SPECS): Use new macros to filter out
    unsupported options taking the default configuration into
    account.
    * gcc.target/sh/pr21255-2-ml.c: Skip if -mb or -m5* is
    specified.  Remove redundant runtime checks.
    * gcc.target/sh/20080410-1.c: Skip if -mb is specified.
    Allow for other than -m4.  Fix typos in comments.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/sh/20080410-1.c
    trunk/gcc/testsuite/gcc.target/sh/pr21255-2-ml.c

[Bug target/50694] SH Target: SH2A little endian does not actually work

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50694

olegendo at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |olegendo at gcc dot gnu.org
         Resolution|                            |FIXED

--- Comment #11 from olegendo at gcc dot gnu.org 2012-02-26 23:24:01 UTC ---
I guess this one is done for now.