Patchwork reorg.c janitor patch 2: handle DEBUG_INSN

login
register
mail settings
Submitter Eric Botcazou
Date Nov. 26, 2012, 5:58 p.m.
Message ID <1830912.piUK0vDJFk@polaris>
Download mbox | patch
Permalink /patch/201998/
State New
Headers show

Comments

Eric Botcazou - Nov. 26, 2012, 5:58 p.m.
> If that is true, then why are there so many post var-tracking passes
> using NONDEBUG_INSN_P and/or looking for the DEBUG_INSN code? See e.g.
> shorten_branches, reorg.c, various machine reorgs, etc.

They are very likely overzealous.

> For example from reorg.c:redundant_insn() in the loop "/* Scan
> backwards looking for a match.  */":
> 
> 151731      nemet       if (!NONDEBUG_INSN_P (trial))
>     99     kenner       continue;
> 151731      nemet       --insns_to_search;
> 
> This was introduced to fix PR41349. Apparently DEBUG_INSN live on
> beyond var-tracking...?

I think that the fix for this PR was really about NOTEs and not DEBUG_INSNs.

I'm going to test the following patchlet on the SPARC:
Richard Sandiford - Nov. 26, 2012, 7:08 p.m.
Eric Botcazou <ebotcazou@adacore.com> writes:
>> If that is true, then why are there so many post var-tracking passes
>> using NONDEBUG_INSN_P and/or looking for the DEBUG_INSN code? See e.g.
>> shorten_branches, reorg.c, various machine reorgs, etc.
>
> They are very likely overzealous.
>
>> For example from reorg.c:redundant_insn() in the loop "/* Scan
>> backwards looking for a match.  */":
>> 
>> 151731      nemet       if (!NONDEBUG_INSN_P (trial))
>>     99     kenner       continue;
>> 151731      nemet       --insns_to_search;
>> 
>> This was introduced to fix PR41349. Apparently DEBUG_INSN live on
>> beyond var-tracking...?
>
> I think that the fix for this PR was really about NOTEs and not DEBUG_INSNs.
>
> I'm going to test the following patchlet on the SPARC:

Steve hit a case where running vartracking before dbr_schedule caused a
bootstrap failure on MIPS.  On targets without delay slots, that kind of
bug has been fixed by:

/* Variable tracking should be run after all optimizations which
   change order of insns.  It also needs a valid CFG.  */
#undef TARGET_DELAY_VARTRACK
#define TARGET_DELAY_VARTRACK true

But obviously we can't do that with dbr_schedule, since it changes
the order of insns but runs with the CFG pulled down.

In the long term it would be good to replace dbr_schedule altogether,
but in the medium term I thought we might want to update it so that
it can run before vartracking.

Richard
Jakub Jelinek - Nov. 26, 2012, 7:12 p.m.
On Mon, Nov 26, 2012 at 07:08:41PM +0000, Richard Sandiford wrote:
> In the long term it would be good to replace dbr_schedule altogether,
> but in the medium term I thought we might want to update it so that
> it can run before vartracking.

var-tracking definitely isn't prepared to handle dbr_schedule created
sequences, I guess it would be much more work than just fixing whatever is
needed on the dbr side that doesn't work right now (if anything).

	Jakub

Patch

Index: reorg.c
===================================================================
--- reorg.c     (revision 193748)
+++ reorg.c     (working copy)
@@ -1628,7 +1628,7 @@  redundant_insn (rtx insn, rtx target, rt
       if (LABEL_P (trial))
        return 0;
 
-      if (!NONDEBUG_INSN_P (trial))
+      if (!INSN_P (trial))
        continue;
       --insns_to_search;
 
@@ -1731,7 +1731,7 @@  redundant_insn (rtx insn, rtx target, rt
        trial && !LABEL_P (trial) && insns_to_search > 0;
        trial = PREV_INSN (trial))
     {
-      if (!NONDEBUG_INSN_P (trial))
+      if (!INSN_P (trial))
        continue;
       --insns_to_search;