Re: [patch RFA middle-end] Fix PR target/41993
On Mon, Nov 12, 2012 at 9:40 AM, Eric Botcazou ebotca...@adacore.com wrote:
It looks to me, that we in fact want:
--cut here--
Index: mode-switching.c
===
--- mode-switching.c(revision 193407)
+++ mode-switching.c(working copy)
@@ -330,7 +330,7 @@
short_block = 1;
break;
}
- if (copy_start = FIRST_PSEUDO_REGISTER)
+ if (!targetm.calls.function_value_regno_p (copy_start))
{
last_insn = return_copy;
continue;
--cut here--
If we find an unrelated HARD register, we will fail in the same way as
described in the PR. This was found by post-reload vzeroupper
insertion pass that tripped on unrelated hard reg assignment. At this
point, we are interested only in hard registers that are also used for
function return value. Actually, even in pre-reload pass, there are no
other assignments to hard registers.
Fine with me if this passes testing on x86-avx and SH4.
Committed to mailine SVN with additional restriction, we only want to
scan instructions that are not debug instructions.
2012-11-13 Uros Bizjak ubiz...@gmail.com
PR target/41993
* mode-switching.c (create_pre_exit): Set return_copy to last_insn if
copy_start is not a function return regno. Skip debug instructions
in instruction scan loop.
Bootstrapped and regression tested on SH4 by Kaz and Oleg, on AVX
target by Vladimir and by me on x86_64-pc-linux-gnu {,-m32 -} AVX
target, configured with --with-arch=corei7-avx --with-cpu=corei7-avx
--enable-languages=all,obj-c++,go with and without postreload
vzeroupper insertion patch.
Uros.
Index: mode-switching.c
===
--- mode-switching.c(revision 193479)
+++ mode-switching.c(working copy)
@@ -242,7 +242,8 @@ create_pre_exit (int n_entities, int *entity_map,
int copy_start, copy_num;
int j;
- if (INSN_P (return_copy))
+ if (INSN_P (return_copy)
+!DEBUG_INSN_P (return_copy))
{
/* When using SJLJ exceptions, the call to the
unregister function is inserted between the
@@ -330,7 +331,7 @@ create_pre_exit (int n_entities, int *entity_map,
short_block = 1;
break;
}
- if (copy_start = FIRST_PSEUDO_REGISTER)
+ if (!targetm.calls.function_value_regno_p (copy_start))
{
last_insn = return_copy;
continue;
Re: [patch RFA middle-end] Fix PR target/41993
On Tue, Nov 13, 2012 at 06:07:20PM +0100, Uros Bizjak wrote: @@ -242,7 +242,8 @@ create_pre_exit (int n_entities, int *entity_map, int copy_start, copy_num; int j; - if (INSN_P (return_copy)) + if (INSN_P (return_copy) + !DEBUG_INSN_P (return_copy)) Please use if (NONDEBUG_INSN_P (return_copy)) instead. Jakub
Re: [patch RFA middle-end] Fix PR target/41993
On Tue, Nov 13, 2012 at 6:09 PM, Jakub Jelinek ja...@redhat.com wrote: int copy_start, copy_num; int j; - if (INSN_P (return_copy)) + if (INSN_P (return_copy) + !DEBUG_INSN_P (return_copy)) Please use if (NONDEBUG_INSN_P (return_copy)) instead. Bah... I did look at this definition and for some reason unknown to me, I didn't see the equivalence. Anyway, I have committed the change. Thanks, Uros.
Re: [patch RFA middle-end] Fix PR target/41993
On Tue, Nov 6, 2012 at 9:12 AM, Eric Botcazou ebotca...@adacore.com wrote:
2012-11-05 Uros Bizjak ubiz...@gmail.com
Kaz Kojima kkoj...@gcc.gnu.org
PR target/41993
* mode-switching.c (create_pre_exit): Set return_copy to
last_insn when copy_start is a pseudo reg.
It looks to me, that we in fact want:
--cut here--
Index: mode-switching.c
===
--- mode-switching.c(revision 193407)
+++ mode-switching.c(working copy)
@@ -330,7 +330,7 @@
short_block = 1;
break;
}
- if (copy_start = FIRST_PSEUDO_REGISTER)
+ if (!targetm.calls.function_value_regno_p (copy_start))
{
last_insn = return_copy;
continue;
--cut here--
If we find an unrelated HARD register, we will fail in the same way as
described in the PR. This was found by post-reload vzeroupper
insertion pass that tripped on unrelated hard reg assignment. At this
point, we are interested only in hard registers that are also used for
function return value. Actually, even in pre-reload pass, there are no
other assignments to hard registers.
Uros.
Re: [patch RFA middle-end] Fix PR target/41993
It looks to me, that we in fact want:
--cut here--
Index: mode-switching.c
===
--- mode-switching.c(revision 193407)
+++ mode-switching.c(working copy)
@@ -330,7 +330,7 @@
short_block = 1;
break;
}
- if (copy_start = FIRST_PSEUDO_REGISTER)
+ if (!targetm.calls.function_value_regno_p (copy_start))
{
last_insn = return_copy;
continue;
--cut here--
If we find an unrelated HARD register, we will fail in the same way as
described in the PR. This was found by post-reload vzeroupper
insertion pass that tripped on unrelated hard reg assignment. At this
point, we are interested only in hard registers that are also used for
function return value. Actually, even in pre-reload pass, there are no
other assignments to hard registers.
Fine with me if this passes testing on x86-avx and SH4.
--
Eric Botcazou
Re: [patch RFA middle-end] Fix PR target/41993
2012-11-05 Uros Bizjak ubiz...@gmail.com
Kaz Kojima kkoj...@gcc.gnu.org
PR target/41993
* mode-switching.c (create_pre_exit): Set return_copy to
last_insn when copy_start is a pseudo reg.
OK, thanks. The number of special cases dealt with in the function is on the
verge of making it barely understandable though. Why couldn't a backward scan
based only on:
/* If the return register is not likely spilled, - as is
the case for floating point on SH4 - then it might
be set by an arithmetic operation that needs a
different mode than the exit block. */
for (j = n_entities - 1; j = 0; j--)
{
int e = entity_map[j];
int mode = MODE_NEEDED (e, return_copy);
if (mode != num_modes[e] mode != MODE_EXIT (e))
break;
}
(with a few shortcuts to speed it up) be sufficient?
--
Eric Botcazou
Re: [patch RFA middle-end] Fix PR target/41993
Eric Botcazou ebotca...@adacore.com wrote:
OK, thanks. The number of special cases dealt with in the function is on the
verge of making it barely understandable though. Why couldn't a backward
scan
based only on:
/* If the return register is not likely spilled, - as is
the case for floating point on SH4 - then it might
be set by an arithmetic operation that needs a
different mode than the exit block. */
for (j = n_entities - 1; j = 0; j--)
{
int e = entity_map[j];
int mode = MODE_NEEDED (e, return_copy);
if (mode != num_modes[e] mode != MODE_EXIT (e))
break;
}
(with a few shortcuts to speed it up) be sufficient?
Although I might be getting you wrong, the current code does a scan
based on those lines but builtin_return, functions with no return
value and exceptions require special treatments and made things
complex.
Regards,
kaz
Re: [patch RFA middle-end] Fix PR target/41993
Although I might be getting you wrong, the current code does a scan based on those lines but builtin_return, functions with no return value and exceptions require special treatments and made things complex. I was referring to the ret_start/reg_end/nregs business: why is it necessary? -- Eric Botcazou
Re: [patch RFA middle-end] Fix PR target/41993
Eric Botcazou ebotca...@adacore.com wrote: I was referring to the ret_start/reg_end/nregs business: why is it necessary? I thought that they are for the return value which requires multiple hard registers. Regards, kaz
Re: [patch RFA middle-end] Fix PR target/41993
On Mon, Nov 5, 2012 at 11:58 PM, Kaz Kojima kkoj...@rr.iij4u.or.jp wrote:
The attached patch is to solve PR target/41993 which will affect
targets using MODE_EXIT.
Without it, we can't find all return registers for __builtin_return
in mode-switching.c:create_pre_exit. See the trail #4 by Uros in
the PR for the details. The patch is tested with bootstrap and
regtested on i686-pc-linux-gnu with no new failures. It's also
tested on cross sh4-unknown-linux-gnu.
Attached patch adds the testcase from PR to the testsuite.
2012-11-06 Uros Bizjak ubiz...@gmail.com
PR middle-end/41993
* gcc.dg/torture/pr41993.c: New test.
Tested on x86_64-pc-linux-gnu {,-m32}, committed to mainline SVN.
Uros.
/* { dg-do compile } */
/* { dg-options -mavx -mvzeroupper { target { i?86-*-* x86_64-*-* } } } */
short retframe_short (void *rframe)
{
__builtin_return (rframe);
}
[patch RFA middle-end] Fix PR target/41993
Hi,
The attached patch is to solve PR target/41993 which will affect
targets using MODE_EXIT.
Without it, we can't find all return registers for __builtin_return
in mode-switching.c:create_pre_exit. See the trail #4 by Uros in
the PR for the details. The patch is tested with bootstrap and
regtested on i686-pc-linux-gnu with no new failures. It's also
tested on cross sh4-unknown-linux-gnu.
OK for trunk?
Regards,
kaz
--
2012-11-05 Uros Bizjak ubiz...@gmail.com
Kaz Kojima kkoj...@gcc.gnu.org
PR target/41993
* mode-switching.c (create_pre_exit): Set return_copy to
last_insn when copy_start is a pseudo reg.
--- ORIG/trunk/gcc/mode-switching.c 2012-11-05 08:07:55.0 +0900
+++ trunk/gcc/mode-switching.c 2012-11-05 19:22:56.0 +0900
@@ -324,7 +324,10 @@ create_pre_exit (int n_entities, int *en
else
break;
if (copy_start = FIRST_PSEUDO_REGISTER)
- break;
+ {
+ last_insn = return_copy;
+ continue;
+ }
copy_num
= hard_regno_nregs[copy_start][GET_MODE (copy_reg)];
