[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 Kazumoto Kojima changed: What|Removed |Added Summary|[4.9/5/6 Regression] [SH] |[5/6 Regression] [SH] Wrong |Wrong code is generated |code is generated with |with stage1 compiler|stage1 compiler --- Comment #32 from Kazumoto Kojima --- (In reply to John Paul Adrian Glaubitz from comment #28) I should have been more clear about these comparison messages. Warning is warning and doesn't cause failure. gcc/cc1*-checksum.o objects depend on the virtual addresses of executables. If randomize-va-space feature of kernel on the target machine is used, then stage2/3 cc1*-checksum.o differs without any compiler problem. These differences are warned but aren't handled as errors for that reason. It looks that the log for 4.8.4-2 includes gcc/d/ctfeexpr.dmd.o differs line just after its 'Bootstrap comparison failure!' line. It looks that that is the cause of failure and is a differnt issue. For the original PR, I've apply the fix on 5/6 in behalf of Oleg after the regtest on trunk which has become possible because 66181 is fixed.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #33 from John Paul Adrian Glaubitz --- (In reply to Kazumoto Kojima from comment #32) > (In reply to John Paul Adrian Glaubitz from comment #28) > I should have been more clear about these comparison messages. > Warning is warning and doesn't cause failure. gcc/cc1*-checksum.o > objects depend on the virtual addresses of executables. If > randomize-va-space feature of kernel on the target machine is used, > then stage2/3 cc1*-checksum.o differs without any compiler problem. > These differences are warned but aren't handled as errors for that reason. Aha, that would explain that, indeed as it. Checking on two of the buildds: root@tirpitz:~> cat /proc/sys/kernel/randomize_va_space 2 root@tirpitz:~> yamashiro:~# cat /proc/sys/kernel/randomize_va_space 2 yamashiro:~# > It looks that the log for 4.8.4-2 includes > gcc/d/ctfeexpr.dmd.o differs > line just after its 'Bootstrap comparison failure!' line. It looks that > that is the cause of failure and is a differnt issue. Ah, thanks for spotting that. Now, should we move this into a new bug report then? > For the original PR, I've apply the fix on 5/6 in behalf of Oleg after > the regtest on trunk which has become possible because 66181 is fixed. Great. Can't wait for the new gcc-5 package to arrive in Debian. Will report back as soon as I know more. Adrian
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #34 from Oleg Endo --- (In reply to John Paul Adrian Glaubitz from comment #33) > > > It looks that the log for 4.8.4-2 includes > > gcc/d/ctfeexpr.dmd.o differs > > line just after its 'Bootstrap comparison failure!' line. It looks that > > that is the cause of failure and is a differnt issue. > > Ah, thanks for spotting that. Now, should we move this into a new bug report > then? If 4.8.something doesn't bootstrap, it would be a "4.8 Regression" type of bug. I'd move it to a new PR.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #35 from John Paul Adrian Glaubitz --- (In reply to Oleg Endo from comment #34) > If 4.8.something doesn't bootstrap, it would be a "4.8 Regression" type of > bug. I'd move it to a new PR. Already. I'll open a new issue then. Just a second. Adrian
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979
--- Comment #36 from Oleg Endo ---
It seems the tstsi peephole is still wrong. While working on AMS the following
example:
int test (char* x, char* y, int z)
{
return ((x[2] & x[3]) == 0) + z;
}
silently produced wrong code by omitting one of the two mem loads. After an
update to the current trunk it results in
sh_tmp.cpp: In function 'int test(char*, char*, int)':
sh_tmp.cpp:6:1: error: insn does not satisfy its constraints:
}
^
(insn 88 69 89 2 (set (reg:SI 1 r1)
(sign_extend:SI (mem:QI (plus:SI (reg/v/f:SI 4 r4 [orig:169 x ] [169])
(const_int 3 [0x3])) [0 MEM[(char *)x_1(D) + 3B]+0 S1
A8]))) sh_tmp.cpp:5 232 {*extendqisi2_compact_mem_disp}
(nil))
which I think is the same as PR 66611. The tstsi related peephole at line
14709 is causing the trouble.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979
--- Comment #37 from Oleg Endo ---
(In reply to Oleg Endo from comment #36)
> It seems the tstsi peephole is still wrong. While working on AMS the
> following example:
>
> int test (char* x, char* y, int z)
> {
> return ((x[2] & x[3]) == 0) + z;
> }
>
> silently produced wrong code by omitting one of the two mem loads. After an
> update to the current trunk it results in
>
> sh_tmp.cpp: In function 'int test(char*, char*, int)':
> sh_tmp.cpp:6:1: error: insn does not satisfy its constraints:
> }
> ^
> (insn 88 69 89 2 (set (reg:SI 1 r1)
> (sign_extend:SI (mem:QI (plus:SI (reg/v/f:SI 4 r4 [orig:169 x ]
> [169])
> (const_int 3 [0x3])) [0 MEM[(char *)x_1(D) + 3B]+0 S1
> A8]))) sh_tmp.cpp:5 232 {*extendqisi2_compact_mem_disp}
> (nil))
>
> which I think is the same as PR 66611. The tstsi related peephole at line
> 14709 is causing the trouble.
The peephole in question can't be used for code sequences that include QImode
or HImode mem loads of the operands. The following fixes the issue by matching
the resulting insn pattern and the constraints while emitting the new peephole
insns. I think it's good to do those full checks as opposed to checking for
the currently known problematic mem loads. This way we an be sure that
whatever the "emit_insn (gen_rtx_SET ..." produces is actually valid code.
The problem exists on the 5 branch and on trunk.
Kaz, could you please add this to your test runs? For me it's a bit difficult
to do proper testing at the moment.
Index: gcc/config/sh/sh.md
===
--- gcc/config/sh/sh.md (revision 224565)
+++ gcc/config/sh/sh.md (working copy)
@@ -14727,8 +14727,19 @@
if (REGNO (operands[1]) == REGNO (operands[2]))
operands[2] = gen_rtx_REG (SImode, REGNO (operands[0]));
- sh_check_add_incdec_notes (emit_insn (gen_rtx_SET (operands[2],
-operands[3])));
+ // We don't know what the new set insn will be in detail. Just make sure
+ // that it still can be recognized and the constraints are satisfied.
+ rtx_insn* i = emit_insn (gen_rtx_SET (operands[2], operands[3]));
+
+ recog_data_d prev_recog_data = recog_data;
+ bool i_invalid = insn_invalid_p (i, false);
+ recog_data = prev_recog_data;
+
+ if (i_invalid)
+FAIL;
+
+ sh_check_add_incdec_notes (i);
+
emit_insn (gen_tstsi_t (operands[2],
gen_rtx_REG (SImode, (REGNO (operands[1]);
})
@@ -14755,8 +14766,19 @@
|| REGNO (operands[2]) == REGNO (operands[5]))"
[(const_int 0)]
{
- sh_check_add_incdec_notes (emit_insn (gen_rtx_SET (operands[2],
-operands[3])));
+ // We don't know what the new set insn will be in detail. Just make sure
+ // that it still can be recognized and the constraints are satisfied.
+ rtx_insn* i = emit_insn (gen_rtx_SET (operands[2], operands[3]));
+
+ recog_data_d prev_recog_data = recog_data;
+ bool i_invalid = insn_invalid_p (i, false);
+ recog_data = prev_recog_data;
+
+ if (i_invalid)
+FAIL;
+
+ sh_check_add_incdec_notes (i);
+
emit_insn (gen_tstsi_t (operands[2],
gen_rtx_REG (SImode, (REGNO (operands[1]);
})
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #38 from Kazumoto Kojima --- (In reply to Oleg Endo from comment #37) > Kaz, could you please add this to your test runs? For me it's a bit > difficult to do proper testing at the moment. I'm testing the patch now. I'll report back when it's done.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #39 from Kazumoto Kojima --- (In reply to Kazumoto Kojima from comment #38) > I'm testing the patch now. I'll report back when it's done. Done. No new failures for the top level "make -k check".
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #40 from John Paul Adrian Glaubitz --- (In reply to Kazumoto Kojima from comment #39) > Done. No new failures for the top level "make -k check". So, chances are gcc-5 would build now?
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #41 from Kazumoto Kojima --- (In reply to John Paul Adrian Glaubitz from comment #40) > So, chances are gcc-5 would build now? Maybe. Trying it with Oleg's patch is a good idea.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #42 from John Paul Adrian Glaubitz --- (In reply to Kazumoto Kojima from comment #41) > Maybe. Trying it with Oleg's patch is a good idea. Is it applied yet? Otherwise I really will have to look into building gcc-5 from SVN myself.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #43 from Kazumoto Kojima --- (In reply to John Paul Adrian Glaubitz from comment #42) > Is it applied yet? Otherwise I really will have to look into building gcc-5 > from SVN myself. It's not.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #44 from Oleg Endo --- Author: olegendo Date: Thu Jun 25 23:12:07 2015 New Revision: 224988 URL: https://gcc.gnu.org/viewcvs?rev=224988&root=gcc&view=rev Log: gcc/ PR target/65979 PR target/66611 * config/sh/sh.md (tstsi_t peephole2): Use insn_invalid_p to check if the replacement insn will work. Modified: trunk/gcc/ChangeLog trunk/gcc/config/sh/sh.md
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #45 from Oleg Endo --- Kaz, I wanted to backport the patch to GCC 5. It doesn't apply because on trunk gen_rtx_SET doesn't take a machine_mode arg. Since SET rtx is always VOIDmode, it has been removed. In GCC 5 though, SImode is used for gen_rtx_SET (see your comment #20). Why is that SImode? It should be VOIDmode, shouldn't it?
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #46 from Kazumoto Kojima --- (In reply to Oleg Endo from comment #45) > Kaz, I wanted to backport the patch to GCC 5. It doesn't apply because on > trunk gen_rtx_SET doesn't take a machine_mode arg. Since SET rtx is always > VOIDmode, it has been removed. In GCC 5 though, SImode is used for > gen_rtx_SET (see your comment #20). Why is that SImode? It should be > VOIDmode, shouldn't it? You are right. I should be VOIDmode, though SImode will work for these cases.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #47 from Oleg Endo --- Author: olegendo Date: Sat Jun 27 00:46:58 2015 New Revision: 225094 URL: https://gcc.gnu.org/viewcvs?rev=225094&root=gcc&view=rev Log: gcc/ Backport from mainline 2015-06-25 Oleg Endo PR target/65979 PR target/66611 * config/sh/sh.md (tstsi_t peephole2): Use insn_invalid_p to check if the replacement insn will work. Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/config/sh/sh.md
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #48 from Oleg Endo --- Can we close this as fixed?
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 --- Comment #49 from John Paul Adrian Glaubitz --- (In reply to Oleg Endo from comment #48) > Can we close this as fixed? Yes.
[Bug target/65979] [5/6 Regression] [SH] Wrong code is generated with stage1 compiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65979 Oleg Endo changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #50 from Oleg Endo --- Fixed.
