Re: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
On 05/24/2015 07:26 PM, Thomas Preud'homme wrote: From: Jeff Law [mailto:l...@redhat.com] Sent: Saturday, May 23, 2015 6:54 AM - if (!can_move_invariant_reg (loop, inv, reg)) + if (!can_move_invariant_reg (loop, inv, dest)) Won't this run into into the same problem if DEST is a SUBREG? One of the very first test in can_move_invariant_reg is: if (!REG_P (reg) || !HARD_REGISTER_P (reg)) return false; So in case of a subreg the insn will not be moved which will execute the same code as before my patch. It would be nicer if it could work with subreg of course but this makes for a much smaller and safer patch. Ah, OK. I was looking at the code prior to the call for can_move_invariant_reg in move_invariant_reg which implies that DEST can be a subreg, but REG can not. But with that check in can_move_invariant_reg obviously won't matter. It feels like we've likely got some dead code here, but that can be a follow-up if you want to pursue. OK for the trunk. Jeff
RE: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
From: Jeff Law [mailto:l...@redhat.com] Sent: Wednesday, May 27, 2015 11:24 PM Ah, OK. I was looking at the code prior to the call for can_move_invariant_reg in move_invariant_reg which implies that DEST can be a subreg, but REG can not. But with that check in can_move_invariant_reg obviously won't matter. It feels like we've likely got some dead code here, but that can be a follow-up if you want to pursue. Are you referring to the subreg code? It's used at the end of the function: inv-reg = reg; inv-orig_regno = regno; OK for the trunk. Thanks, committed. Best regards, Thomas
RE: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
From: Jeff Law [mailto:l...@redhat.com] Sent: Saturday, May 23, 2015 6:54 AM - if (!can_move_invariant_reg (loop, inv, reg)) + if (!can_move_invariant_reg (loop, inv, dest)) Won't this run into into the same problem if DEST is a SUBREG? One of the very first test in can_move_invariant_reg is: if (!REG_P (reg) || !HARD_REGISTER_P (reg)) return false; So in case of a subreg the insn will not be moved which will execute the same code as before my patch. It would be nicer if it could work with subreg of course but this makes for a much smaller and safer patch. Best regards, Thomas
Re: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
On 05/20/2015 08:04 PM, Thomas Preud'homme wrote: From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme From: Steven Bosscher [mailto:stevenb@gmail.com] Sent: Tuesday, May 19, 2015 7:21 PM Not OK. This will break in move_invariants() when it looks at REGNO (inv-reg). Indeed. I'm even surprised all tests passed. Ok I will just prevent moving in such a case. I'm running the tests now and will get back to you tomorrow. Patch is now tested via bootstrap + testsuite run on x86_64-linux-gnu and building arm-none-eabi cross-compiler + testsuite run. Both testsuite run show no regression. diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c index 76a009f..4ce3576 100644 --- a/gcc/loop-invariant.c +++ b/gcc/loop-invariant.c @@ -1626,7 +1626,7 @@ move_invariant_reg (struct loop *loop, unsigned invno) if (REG_P (reg)) regno = REGNO (reg); - if (!can_move_invariant_reg (loop, inv, reg)) + if (!can_move_invariant_reg (loop, inv, dest)) Won't this run into into the same problem if DEST is a SUBREG? Jeff
RE: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
From: Steven Bosscher [mailto:stevenb@gmail.com] Sent: Tuesday, May 19, 2015 7:21 PM Not OK. This will break in move_invariants() when it looks at REGNO (inv-reg). Indeed. I'm even surprised all tests passed. Ok I will just prevent moving in such a case. I'm running the tests now and will get back to you tomorrow. Best regards, Thomas
RE: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme
From: Steven Bosscher [mailto:stevenb@gmail.com]
Sent: Tuesday, May 19, 2015 7:21 PM
Not OK.
This will break in move_invariants() when it looks at REGNO (inv-reg).
Indeed. I'm even surprised all tests passed. Ok I will just prevent moving
in such a case. I'm running the tests now and will get back to you
tomorrow.
Patch is now tested via bootstrap + testsuite run on x86_64-linux-gnu and
building arm-none-eabi cross-compiler + testsuite run. Both testsuite run
show no regression.
diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index 76a009f..4ce3576 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -1626,7 +1626,7 @@ move_invariant_reg (struct loop *loop, unsigned invno)
if (REG_P (reg))
regno = REGNO (reg);
- if (!can_move_invariant_reg (loop, inv, reg))
+ if (!can_move_invariant_reg (loop, inv, dest))
{
reg = gen_reg_rtx_and_attrs (dest);
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr66168.c
b/gcc/testsuite/gcc.c-torture/compile/pr66168.c
new file mode 100644
index 000..d6bfc7b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr66168.c
@@ -0,0 +1,15 @@
+int a, b;
+
+void
+fn1 ()
+{
+ for (;;)
+{
+ for (b = 0; b 3; b++)
+ {
+ char e[2];
+ char f = e[1];
+ a ^= f ? 1 / f : 0;
+ }
+}
+}
Ok for trunk?
Best regards,
Thomas
Re: [PATCH] Fix PR66168: ICE due to incorrect invariant register info
On Tue, May 19, 2015 at 12:17 PM, Thomas Preud'homme wrote: 2015-05-18 Thomas Preud'homme PR rtl-optimization/66168 * loop-invariant.c (move_invariant_reg): Set inv-reg to destination of inv-insn when moving an invariant without introducing a temporary register. Not OK. This will break in move_invariants() when it looks at REGNO (inv-reg). Ciao! Steven
