diff mbox

[6/9] sel-sched: Don't mess with register restores

Message ID 38963d815ea81b55d5b8e2ed85f2b347aad21395.1470015604.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Aug. 1, 2016, 1:42 a.m. UTC
If selective scheduling copies register restores it confuses dwarf2cfi.

2016-06-07  Segher Boessenkool  <segher@kernel.crashing.org>

	* sel-sched-ir.c (init_global_and_expr_for_insn): Don't copy
	instructions with a REG_CFA_RESTORE note.
---
 gcc/sel-sched-ir.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrey Belevantsev Aug. 4, 2016, 7:33 a.m. UTC | #1
Hello,

On 01.08.2016 4:42, Segher Boessenkool wrote:
> If selective scheduling copies register restores it confuses dwarf2cfi.
> 
> 2016-06-07  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	* sel-sched-ir.c (init_global_and_expr_for_insn): Don't copy
> 	instructions with a REG_CFA_RESTORE note.

OK from sel-sched POV.

Best,
Andrey

> ---
>  gcc/sel-sched-ir.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
> index 83f813a..4a3984a 100644
> --- a/gcc/sel-sched-ir.c
> +++ b/gcc/sel-sched-ir.c
> @@ -3015,6 +3015,7 @@ init_global_and_expr_for_insn (insn_t insn)
>            /* TRAP_IF though have an INSN code is control_flow_insn_p ().  */
>            || control_flow_insn_p (insn)
>            || volatile_insn_p (PATTERN (insn))
> +	  || find_reg_note (insn, REG_CFA_RESTORE, NULL)
>            || (targetm.cannot_copy_insn_p
>                && targetm.cannot_copy_insn_p (insn)))
>          force_unique_p = true;
>
Jeff Law Sept. 8, 2016, 5:54 p.m. UTC | #2
On 07/31/2016 07:42 PM, Segher Boessenkool wrote:
> If selective scheduling copies register restores it confuses dwarf2cfi.
>
> 2016-06-07  Segher Boessenkool  <segher@kernel.crashing.org>
>
> 	* sel-sched-ir.c (init_global_and_expr_for_insn): Don't copy
> 	instructions with a REG_CFA_RESTORE note.
Similarly, I think you're papering over a lifetime problem of some kind 
here.

jeff
Segher Boessenkool Sept. 9, 2016, 9:13 p.m. UTC | #3
On Thu, Sep 08, 2016 at 11:54:33AM -0600, Jeff Law wrote:
> On 07/31/2016 07:42 PM, Segher Boessenkool wrote:
> >If selective scheduling copies register restores it confuses dwarf2cfi.
> >
> >2016-06-07  Segher Boessenkool  <segher@kernel.crashing.org>
> >
> >	* sel-sched-ir.c (init_global_and_expr_for_insn): Don't copy
> >	instructions with a REG_CFA_RESTORE note.
> Similarly, I think you're papering over a lifetime problem of some kind 
> here.

sel-sched-ir.c says

    /* Certain instructions cannot be cloned, and frame related insns and
       the insn adjacent to NOTE_INSN_EPILOGUE_BEG cannot be moved out of
       their block.  */
    if (prologue_epilogue_contains (insn))

...

and I'm just extending that to "epilogue" instructions not in the
"epilogue" ;-)

If all such epilogue instructions always had a REG_CFA_RESTORE note,
we could drop the "old" thing; but even instructions restoring a register
do not always have such a note (they can be batched up, and they can be
not emitted at all if not shrink-wrapping).


Segher
Jeff Law Sept. 12, 2016, 5:24 p.m. UTC | #4
On 09/09/2016 03:13 PM, Segher Boessenkool wrote:
> On Thu, Sep 08, 2016 at 11:54:33AM -0600, Jeff Law wrote:
>> On 07/31/2016 07:42 PM, Segher Boessenkool wrote:
>>> If selective scheduling copies register restores it confuses dwarf2cfi.
>>>
>>> 2016-06-07  Segher Boessenkool  <segher@kernel.crashing.org>
>>>
>>> 	* sel-sched-ir.c (init_global_and_expr_for_insn): Don't copy
>>> 	instructions with a REG_CFA_RESTORE note.
>> Similarly, I think you're papering over a lifetime problem of some kind
>> here.
>
> sel-sched-ir.c says
>
>     /* Certain instructions cannot be cloned, and frame related insns and
>        the insn adjacent to NOTE_INSN_EPILOGUE_BEG cannot be moved out of
>        their block.  */
>     if (prologue_epilogue_contains (insn))
>
> ...
>
> and I'm just extending that to "epilogue" instructions not in the
> "epilogue" ;-)
>
> If all such epilogue instructions always had a REG_CFA_RESTORE note,
> we could drop the "old" thing; but even instructions restoring a register
> do not always have such a note (they can be batched up, and they can be
> not emitted at all if not shrink-wrapping).
Can you fix this by registering the separate prologue/epilogue insns in 
prologue_insn_hash and epilogue_insn_hash or does that have unintended 
consequences?


Jeff
Segher Boessenkool Sept. 14, 2016, 1:23 p.m. UTC | #5
On Mon, Sep 12, 2016 at 11:24:01AM -0600, Jeff Law wrote:
> >sel-sched-ir.c says
> >
> >    /* Certain instructions cannot be cloned, and frame related insns and
> >       the insn adjacent to NOTE_INSN_EPILOGUE_BEG cannot be moved out of
> >       their block.  */
> >    if (prologue_epilogue_contains (insn))
> >
> >...
> >
> >and I'm just extending that to "epilogue" instructions not in the
> >"epilogue" ;-)
> >
> >If all such epilogue instructions always had a REG_CFA_RESTORE note,
> >we could drop the "old" thing; but even instructions restoring a register
> >do not always have such a note (they can be batched up, and they can be
> >not emitted at all if not shrink-wrapping).
> Can you fix this by registering the separate prologue/epilogue insns in 
> prologue_insn_hash and epilogue_insn_hash or does that have unintended 
> consequences?

An interesting plan, I'll try.


Segher
diff mbox

Patch

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 83f813a..4a3984a 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3015,6 +3015,7 @@  init_global_and_expr_for_insn (insn_t insn)
           /* TRAP_IF though have an INSN code is control_flow_insn_p ().  */
           || control_flow_insn_p (insn)
           || volatile_insn_p (PATTERN (insn))
+	  || find_reg_note (insn, REG_CFA_RESTORE, NULL)
           || (targetm.cannot_copy_insn_p
               && targetm.cannot_copy_insn_p (insn)))
         force_unique_p = true;