Bug 32497 - Crosscomiling native sh3 gcc on a 64-bit host fails
Summary: Crosscomiling native sh3 gcc on a 64-bit host fails
Status: WAITING
Alias: None
Product: gcc
Classification: Unclassified
Component: bootstrap (show other bugs)
Version: 4.1.3
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: build
Depends on:
Blocks:
 
Reported: 2007-06-25 14:33 UTC by Valeriy E. Ushakov
Modified: 2020-12-07 22:25 UTC (History)
5 users (show)

See Also:
Host: shle--netbsdelf
Target: shle--netbsdelf
Build: x86_64--netbsd
Known to work:
Known to fail:
Last reconfirmed: 2016-10-03 00:00:00


Attachments
Suggested fix (837 bytes, patch)
2008-04-17 20:14 UTC, Valeriy E. Ushakov
Details | Diff
Diff against gcc-6.4.0 (501 bytes, patch)
2018-09-07 15:59 UTC, Valeriy E. Ushakov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Valeriy E. Ushakov 2007-06-25 14:33:51 UTC
NetBSD autobuild cluster is a mixture of i386 and amd64 machines, and
when sh3 builds hit an amd64 build host the build fails when
cross-compiling native /usr/bin/gcc for sh3 target.

/usr/nb/tools/bin/shle--netbsdelf-gcc ... -c insn-emit.c
insn-emit.c: In function 'gen_mshfhi_l_di':
insn-emit.c:5476: warning: integer constant is too large for 'long' type
insn-emit.c: In function 'gen_mshflo_l_di':
insn-emit.c:5493: warning: this decimal constant is unsigned only in ISO C90
--- insn-emit.o ---
*** [insn-emit.o] Error code 1
1 error

nbmake: stopped in /home/builds/ab/HEAD/src/gnu/usr.bin/gcc4/backend

My i386 build (this one is ok) has:

/* /usr/src/gnu/dist/gcc4/gcc/config/sh/sh.md:12291 */
rtx
gen_mshfhi_l_di (rtx operand0 ATTRIBUTE_UNUSED,
        rtx operand1 ATTRIBUTE_UNUSED,
        rtx operand2 ATTRIBUTE_UNUSED)
{
  return gen_rtx_SET (VOIDmode,
        operand0,
        gen_rtx_IOR (DImode,
        gen_rtx_LSHIFTRT (DImode,
        operand1,
        const_int_rtx[MAX_SAVED_CONST_INT + (32)]),
        gen_rtx_AND (DImode,
        operand2,
        GEN_INT (-4294967296LL))));
}

/* /usr/src/gnu/dist/gcc4/gcc/config/sh/sh.md:12343 */
rtx
gen_mshflo_l_di (rtx operand0 ATTRIBUTE_UNUSED,
        rtx operand1 ATTRIBUTE_UNUSED,
        rtx operand2 ATTRIBUTE_UNUSED)
{
  return gen_rtx_SET (VOIDmode,
        operand0,
        gen_rtx_IOR (DImode,
        gen_rtx_AND (DImode,
        operand1,
        GEN_INT (4294967295LL)),
        gen_rtx_ASHIFT (DImode,
        operand2,
        const_int_rtx[MAX_SAVED_CONST_INT + (32)])));
}

I don't have any 64-bit machine handy, but a friend sent me
insn-emit.c (failing like above) from his 64-bit machine and it has
4294967295 and -4294967296 in the above marked as long (L),
not long long (LL).  And as that insn-emit.c is compiled with
shle--netbsdelf-gcc it is not happy about those constants.

While this issue was discussed on NetBSD mailing lists
the following comment was made by Izumi Tsutsui:

| The statements in insn-emit.c are generated by
| the following line:
| >       printf (HOST_WIDE_INT_PRINT_DEC_C, INTVAL (x));
| in src/gnu/dist/gcc4/gcc/genemit.c, and HOST_WIDE_INT_PRINT_DEC_C
| is defined in src/gnu/dist/gcc4/gcc/hwint.h.
[...]
| Sources for the "build" machine should include "bconfig.h"
| then HOST_foo macros will represent build's sizes.
| Sources for the "host" machine should include "config.h"
| then HOST_foo macros will represent host's sizes.
|
| But in genemit.c case, it's a source for the build so
| it includes "bconfig.h" then HOST_foo macros represent
| build's size, but HOST_WIDE_INT_PRINT_DEC_C is used
| in printf(3) to create insn-emit.c which is a source
| of the host machine.
| Then a build's type (long) is used in a host's source
| where it should be a host's type (long long).
|
| Is there any way to refer host's sizes ("LL" suffix
| for 64bit integers in this case) in build's sources?
Comment 1 Valeriy E. Ushakov 2008-04-17 20:14:53 UTC
Created attachment 15492 [details]
Suggested fix

Attached patch makes NetBSD/landisk (sh4) cross-build complete successfully on NetBSD/amd64, where it used to fail before.  Native gcc built as part of the system (host sh3 -> target sh3) works and produces binaries identical to binaries produced by the native compiler w/out this patch (cross-compiled from i386).

The patch basically defers the choice of L/LL suffix until the moment when insn-*.c is compiled with correct HOST_WIDE* defines (gen*.c uses build machine HOST_WIDE* values, not target machine).

There's still the problem (marked with XXX in the patch) with the minimim host wide decimal constant that I'm not sure how to address properly, short of always generating -1/+1 hack for all negative values.
Comment 2 Andrew Pinski 2016-10-03 06:02:28 UTC
Does this work with a newer GCC?
Comment 3 Valeriy E. Ushakov 2016-10-03 09:40:48 UTC
The patch above has been committed to the NetBSD imported version of gcc back in April 2008 and has been merged in all the new imported versions ever since.

It's been quite a few years so I don't remember most details, however I assume the patch is still relevant.  I will try to back it out and see what happens.
Comment 4 Valeriy E. Ushakov 2016-10-03 10:55:49 UTC
Seems to fail more or less the same way without the patch.  NetBSD current with gcc 5.4.0.  Building on linux/amd64.  When native gcc is cross compiled

shle--netbsdelf-c++ ... -c insn-emit.c

fails with:

insn-emit.c:6382:3: error: this decimal constant is unsigned only in ISO C90 [-Werror]

This is about 4294967295L in gen_mshflo_l_di().   It no longer complains about -4294967296L in gen_mshfhi_l_di() just above, though.  The relevant fragment of insn-emit.c is:

/* /home/uwe/work/netbsd/ro/src/external/gpl3/gcc/dist/gcc/config/sh/sh.md:15987 */
rtx
gen_mshfhi_l_di (rtx operand0 ATTRIBUTE_UNUSED,
        rtx operand1 ATTRIBUTE_UNUSED,
        rtx operand2 ATTRIBUTE_UNUSED)
{
  return gen_rtx_SET (VOIDmode,
        operand0,
        gen_rtx_IOR (DImode,
        gen_rtx_LSHIFTRT (DImode,
        operand1,
        const_int_rtx[MAX_SAVED_CONST_INT + (32)]),
        gen_rtx_AND (DImode,
        operand2,
        GEN_INT (-4294967296L))));
}

/* /home/uwe/work/netbsd/ro/src/external/gpl3/gcc/dist/gcc/config/sh/sh.md:16037 */
rtx
gen_mshflo_l_di (rtx operand0 ATTRIBUTE_UNUSED,
        rtx operand1 ATTRIBUTE_UNUSED,
        rtx operand2 ATTRIBUTE_UNUSED)
{
  return gen_rtx_SET (VOIDmode,
        operand0,
        gen_rtx_IOR (DImode,
        gen_rtx_AND (DImode,
        operand1,
        GEN_INT (4294967295L)),
        gen_rtx_ASHIFT (DImode,
        operand2,
        const_int_rtx[MAX_SAVED_CONST_INT + (32)])));
}
Comment 5 Oleg Endo 2016-10-03 12:13:56 UTC
(In reply to Valeriy E. Ushakov from comment #4)

>         GEN_INT (-4294967296L))));

>         GEN_INT (4294967295L)),

I think those should be -4294967296LL and 4294967295LL.  Do you have that build setup at hand to try this out?

BTW, those two patterns do not exist anymore in trunk, as SH5 stuff has been removed.
Comment 6 Valeriy E. Ushakov 2016-10-03 15:02:08 UTC
(In reply to Oleg Endo from comment #5)
> (In reply to Valeriy E. Ushakov from comment #4)
> 
> >         GEN_INT (-4294967296L))));
> 
> >         GEN_INT (4294967295L)),
> 
> I think those should be -4294967296LL and 4294967295LL.

Right, with the patch above (committed in NetBSD) I get the following diff between insn-emit.c:

-       GEN_INT (-4294967296L))));
+       GEN_INT (HOST_WIDE_INT_CONSTANT (-4294967296)))));

... etc

so, effectively, it removes the L/LL decision from genemit.c and delays it until insn-emit.c is compiled.


> Do you have that build setup at hand to try this out?

I guess the easiest way to get out NetBSD source tree and to cross-build say NetBSD/landisk on a 64 bit host (any linux/amd64 will do, osx probably too, but there were a couple of pitfalls recently).

You can grab sources from e.g. http://ftp.netbsd.org/pub/NetBSD/NetBSD-7.0.1/source/sets/ and consult BUILDING, but in a nutshell something like

./build.sh -m landisk -j8 -u -U distribution

(where -j8 is the amount of parallel jobs you want).
You can mail me privately if you need any help with this.


> BTW, those two patterns do not exist anymore in trunk, as SH5 stuff has been
> removed.

Yes, but that just masks the bug that L/LL selection is done in wrong context.
Comment 7 Eric Gallager 2018-09-07 01:38:24 UTC
(In reply to Valeriy E. Ushakov from comment #6)
> (In reply to Oleg Endo from comment #5)
> > BTW, those two patterns do not exist anymore in trunk, as SH5 stuff has been
> > removed.
> 
> Yes, but that just masks the bug that L/LL selection is done in wrong
> context.

OK, so what would be a better way to word the title then, such that it doesn't mention an obsolete target?
Comment 8 Valeriy E. Ushakov 2018-09-07 07:51:27 UTC
I don't understand.  The title mentions sh3, which is not obsolete.

It's been 11 (eleven!) years.  As far as I know this patch (adjusted to catch up with the changes, but essentially the same) is still necessary for gcc 6.4 that is the compiler in NetBSD-current.
Comment 9 Oleg Endo 2018-09-07 11:26:00 UTC
(In reply to Valeriy E. Ushakov from comment #8)
> 
> It's been 11 (eleven!) years.  As far as I know this patch (adjusted to
> catch up with the changes, but essentially the same) is still necessary for
> gcc 6.4 that is the compiler in NetBSD-current.

The patch contains changes to things outside the SH backend, so I can't approve it.  Can you please send the most recent version of the patch to the patches ML so that other people can review it?
Comment 10 Eric Gallager 2018-09-07 13:54:06 UTC
(In reply to Valeriy E. Ushakov from comment #8)
> I don't understand.  The title mentions sh3, which is not obsolete.
> 

Sorry, I misunderstood the sh numbering system when asking that last night (it was late and I was tired)
Comment 11 Valeriy E. Ushakov 2018-09-07 15:59:12 UTC
Created attachment 44668 [details]
Diff against gcc-6.4.0

This is essentially the same diff except gcc now provides its own HOST_WIDE_INT_C() macro, so the patch uses that instead of defining its own.
Comment 12 Valeriy E. Ushakov 2018-09-07 16:06:08 UTC
I've attached updated patch against gcc 6.4.0.  If I un-apply that patch to the NetBSD tree with patch -R (i.e. revert the files to their original state as in gcc 6.4.0) I get

$ nbmake-landisk insn-emit.o
#   compile  backend/insn-emit.o
/home/uwe/work/netbsd/build/tools/bin/shle--netbsdelf-c++ -Os -freorder-blocks -Wall -Wpointer-arith -Wno-sign-compare -Wa,--fatal-warnings -Wno-uninitialized -Wno-maybe-uninitialized -Werror   -fPIE -Wno-narrowing -Wno-unused -std=gnu++98 -Wno-stack-protector -fno-exceptions -fno-rtti -fasynchronous-unwind-tables   -I. -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/usr.bin/backend/../gcc/arch/sh3el -DIN_GCC -DHAVE_CONFIG_H -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/. -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../include -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libcpp/include -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libdecnumber -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libdecnumber/dpd -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libbacktrace -DTARGET_NAME=\"shle--netbsdelf\" -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/libgcc -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/usr.bin/backend/../../lib/libgcc/libgcov/arch/sh3el --sysroot=/home/uwe/work/netbsd/build/distrib/landisk -DLOCALEDIR=\"/usr/share/locale\" -DNETBSD_NATIVE -I. -DENABLE_SHARED_LIBGCC  -c    insn-emit.c -o insn-emit.o
insn-emit.c:6346:3: error: this decimal constant is unsigned only in ISO C90 [-Werror]
   return gen_rtx_SET (operand0,
   ^~~~~~
cc1plus: all warnings being treated as errors
[...]
$ cat -n insn-emit.c | sed -n '6341,6354p'
  6341  rtx
  6342  gen_mshflo_l_di (rtx operand0 ATTRIBUTE_UNUSED,
  6343          rtx operand1 ATTRIBUTE_UNUSED,
  6344          rtx operand2 ATTRIBUTE_UNUSED)
  6345  {
  6346    return gen_rtx_SET (operand0,
  6347          gen_rtx_IOR (DImode,
  6348          gen_rtx_AND (DImode,
  6349          operand1,
  6350          GEN_INT (4294967295L)),
  6351          gen_rtx_ASHIFT (DImode,
  6352          operand2,
  6353          const_int_rtx[MAX_SAVED_CONST_INT + (32)])));
  6354  }

and

$ nbmake-landisk insn-recog.o
#   compile  backend/insn-recog.o
/home/uwe/work/netbsd/build/tools/bin/shle--netbsdelf-c++ -Os -freorder-blocks -Wall -Wpointer-arith -Wno-sign-compare -Wa,--fatal-warnings -Wno-uninitialized -Wno-maybe-uninitialized -Werror   -fPIE -Wno-narrowing -Wno-unused -std=gnu++98 -Wno-stack-protector -fno-exceptions -fno-rtti -fasynchronous-unwind-tables   -I. -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/usr.bin/backend/../gcc/arch/sh3el -DIN_GCC -DHAVE_CONFIG_H -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/. -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../include -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libcpp/include -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libdecnumber -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libdecnumber/dpd -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/../libbacktrace -DTARGET_NAME=\"shle--netbsdelf\" -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/libgcc -I/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/usr.bin/backend/../../lib/libgcc/libgcov/arch/sh3el --sysroot=/home/uwe/work/netbsd/build/distrib/landisk -DLOCALEDIR=\"/usr/share/locale\" -DNETBSD_NATIVE -I. -DENABLE_SHARED_LIBGCC  -c    insn-recog.c -o insn-recog.o
insn-recog.c:1532:7: error: this decimal constant is unsigned only in ISO C90 [-Werror]
       || XWINT (x4, 0) != -2147483648L
       ^~
insn-recog.c:5762:5: error: this decimal constant is unsigned only in ISO C90 [-Werror]
     case -2147483648L:
     ^~~~
/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/config/sh/sh.md:15774:15: error: this decimal constant is unsigned only in ISO C90 [-Werror]
   DONE;
               ^ 
/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/config/sh/sh.md:4832:17: error: this decimal constant is unsigned only in ISO C90 [-Werror]
       (and:SI (match_dup 1) (const_int 1))))
                 ^~~~
/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/config/sh/sh.md:16430:17: error: this decimal constant is unsigned only in ISO C90 [-Werror]
 (define_split
                 ^   
/home/uwe/work/netbsd/ro/src/external/gpl3/gcc.old/dist/gcc/config/sh/sh.md:14012:15: error: this decimal constant is unsigned only in ISO C90 [-Werror]
   [(set (match_operand:DF 0 "fp_arith_reg_operand" "=f")
               ^~
cc1plus: all warnings being treated as errors
Comment 13 Valeriy E. Ushakov 2018-09-07 16:20:09 UTC
The above build was done on a linux/amd64 host.

The error happens when NetBSD build cross-compiles native NetBSD/sh3 gcc, i.e. the compiler that will run natively on sh3 as part of the NetBSD distribution.

I'm sorry, but I don't have time or energy to come up with a standalone reproducer, especially given prior history of this bug report being ignored for over a decade.  Someone familiar with gcc code base should be able to do that easily I think.  And as I said in comment #5, you can reproduce it with the NetBSD source tree with a single command on about any modern unix on a 64-bit host.

I guess linux-sh folks have never run into this because they use cross-compiler exclusively and never compile stuff on sh3 itself, so they never build native gcc for sh3.
Comment 14 Valeriy E. Ushakov 2018-09-07 16:22:02 UTC
Sorry, I meant comment #6 in the above.
Comment 15 Eric Gallager 2018-12-07 03:51:05 UTC
(In reply to Valeriy E. Ushakov from comment #11)
> Created attachment 44668 [details]
> Diff against gcc-6.4.0
> 
> This is essentially the same diff except gcc now provides its own
> HOST_WIDE_INT_C() macro, so the patch uses that instead of defining its own.

can you please send it to the gcc-patches mailing list for review?
Comment 16 Valeriy E. Ushakov 2018-12-07 10:21:31 UTC
(In reply to Eric Gallager from comment #15)
> (In reply to Valeriy E. Ushakov from comment #11)
> > Created attachment 44668 [details]
> > Diff against gcc-6.4.0
> > 
> > This is essentially the same diff except gcc now provides its own
> > HOST_WIDE_INT_C() macro, so the patch uses that instead of defining its own.
> 
> can you please send it to the gcc-patches mailing list for review?

This patch has been sitting in your bugtracker for 11 years.  Anything I know about this bug is written in this bug report and swapped out of my active memory, so I cannot meaningfully answer any questions about that patch on gcc-patches@ other than by referring people to what's written here in this bug report.   Why do I have to go through this strange ritual of taking this patch out of gcc's own bugtracker and sending it to gcc's own list for proposed patches?  This is not some proposed change that I can meaningfully advocate (like a new feature or something).

I.e. what that action is going to change from the standpoint of communication setup?  As I see it, it can only make things worse b/c if I'm actually asked any questions I can't answer them, or someone replies to that patch only to the mailing list and I miss that reply (and it's not recorded here too).  And then it will just look as my fault, not engaging in a proper discussion.
Comment 17 Eric Gallager 2019-03-07 06:41:25 UTC
(In reply to Valeriy E. Ushakov from comment #16)
> (In reply to Eric Gallager from comment #15)
> > (In reply to Valeriy E. Ushakov from comment #11)
> > > Created attachment 44668 [details]
> > > Diff against gcc-6.4.0
> > > 
> > > This is essentially the same diff except gcc now provides its own
> > > HOST_WIDE_INT_C() macro, so the patch uses that instead of defining its own.
> > 
> > can you please send it to the gcc-patches mailing list for review?
> 
> This patch has been sitting in your bugtracker for 11 years.  Anything I
> know about this bug is written in this bug report and swapped out of my
> active memory, so I cannot meaningfully answer any questions about that
> patch on gcc-patches@ other than by referring people to what's written here
> in this bug report.   Why do I have to go through this strange ritual of
> taking this patch out of gcc's own bugtracker and sending it to gcc's own
> list for proposed patches?  This is not some proposed change that I can
> meaningfully advocate (like a new feature or something).
> 
> I.e. what that action is going to change from the standpoint of
> communication setup?  As I see it, it can only make things worse b/c if I'm
> actually asked any questions I can't answer them, or someone replies to that
> patch only to the mailing list and I miss that reply (and it's not recorded
> here too).  And then it will just look as my fault, not engaging in a proper
> discussion.

Don't ask me, I didn't create the rule... just trying to help move things along here...
Comment 18 Thomas Koenig 2019-03-07 09:23:39 UTC
(In reply to Valeriy E. Ushakov from comment #16)
> Why do I have to go through this strange ritual of
> taking this patch out of gcc's own bugtracker and sending it to gcc's own
> list for proposed patches?

I am knowledge-free about this particular area, just wanted to make
some general comments.

Several reasons, some of them overlapping.

This is how gcc development operates.  For a large project like gcc
with many volunteers, you need to have rules, and in general, people
should stick to them.

Posting to gcc-patches gives everybody a chance to look at patches
and speak up if they have any concerns or advice.  Not very many
people read individual PRs.

Finally, it's a very good thing that you need approval from somebody.
Compilers are notoriously complex beasts, and regressions for some
corner case can creep in quite easily - less easily if somebody
else has a look.

Of course, another thing that can be done is to add a maintainer for
a particular area to the PR. I have done so with this.