public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFA PR middle-end/48770
@ 2011-05-27 19:10 Jeff Law
  2011-05-30 12:34 ` Bernd Schmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2011-05-27 19:10 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Updated based on some comments from Bernd; specifically the other use of
delete_dead_insn has been removed.

WRT the assembly differences on MIPS Bernd referred to; what ultimately
caused this problem were two dead insns that had been previously
eliminated by reload were still in the insn stream and inhibited an
if-conversion which resulted in slightly different assembly code, but
shouldn't have had any significant impact on the performance or size of
the resulting code.  The dead insns were deleted by the post-reload DCE
pass (which obviously runs after post-reload if conversion).

If we really wanted to get those insns out of the stream, we could flag
when reload deleted insns which might result in dead code remaining in
the stream, then conditionally run DCE immediately after reload.  I
didn't think this was worth doing right now, but if someone objects I
can certainly look into it.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.

Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN3+RCAAoJEBRtltQi2kC7qUMH/3Z8NB8bogEnVvS7qF6eubu0
UNWp2HzJ/QnvxgKQiJUBar/kOF3y20kl2oZpqIoPVea2TqmeZ/lfuS1+lzWWxC/t
HGvjBWfkA60DkcDxFmHn0TadoqL11eAWEzsRRf/HfEn6I3rnrO3vP0RR3X8Dtiqe
qeFf8HVY8ysQoaFTyaiMLLo7yePmvlVy+HTsg+s3F0Yvg18P2WoJjpM/EY2y/tUR
QmIcFi8q4Q92YsfBpjweY6NyqepKrATAZT+CcFdc7GFvfa5TZQj9gGhWqPHiHQ5w
3c8RmKbaGEWhPSjIgmcqtY/FpY2CoRDn2BMosr8N4F0FMN2PkQlU8E7hTE2V6eM=
=UeKy
-----END PGP SIGNATURE-----

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 5261 bytes --]

	PR middle-end/48770
	* ira.c (update_equiv_regs): Update comment.
	* reload1.c (reload): Don't call delete_dead_insn to delete
	insns which create equivalences.
	(eliminate_regs_in_insn): Likewise.
	(delete_dead_insn): Remove.

	PR middle-end/48770
	* gcc.dg/pr48770.c: New test.

Index: testsuite/gcc.dg/pr48770.c
===================================================================
*** testsuite/gcc.dg/pr48770.c	(revision 0)
--- testsuite/gcc.dg/pr48770.c	(revision 0)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do run } */
+ /* { dg-options "-O -fprofile-arcs -fPIC -fno-dce -fno-forward-propagate" } */
+ 
+ int test_goto2 (int f)
+ {
+   int i;
+   for (i = 0; ({_Bool a = i < 10;a;}); i++)
+   {
+     if (i == f)
+       goto lab2;
+   }
+   return 4;
+ lab2:
+   return 8;
+ }
+ 
+ int main ()
+ {
+   test_goto2 (30);
+   return 0;
+ }
Index: ira.c
===================================================================
*** ira.c	(revision 174066)
--- ira.c	(working copy)
*************** update_equiv_regs (void)
*** 2842,2848 ****
  	     If we don't have a REG_EQUIV note, see if this insn is loading
  	     a register used only in one basic block from a MEM.  If so, and the
  	     MEM remains unchanged for the life of the register, add a REG_EQUIV
! 	     note.  */
  
  	  note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
  
--- 2842,2848 ----
  	     If we don't have a REG_EQUIV note, see if this insn is loading
  	     a register used only in one basic block from a MEM.  If so, and the
  	     MEM remains unchanged for the life of the register, add a REG_EQUIV
! 	     note, this creates a block local equivalence.  */
  
  	  note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
  
Index: reload1.c
===================================================================
*** reload1.c	(revision 174066)
--- reload1.c	(working copy)
*************** static void delete_caller_save_insns (vo
*** 356,362 ****
  
  static void spill_failure (rtx, enum reg_class);
  static void count_spilled_pseudo (int, int, int);
- static void delete_dead_insn (rtx);
  static void alter_reg (int, int, bool);
  static void set_label_offsets (rtx, rtx, int);
  static void check_eliminable_occurrences (rtx);
--- 356,361 ----
*************** reload (rtx first, int global)
*** 1011,1021 ****
  	mark_elimination (ep->from, ep->to);
  
    /* If a pseudo has no hard reg, delete the insns that made the equivalence.
!      If that insn didn't set the register (i.e., it copied the register to
!      memory), just delete that insn instead of the equivalencing insn plus
!      anything now dead.  If we call delete_dead_insn on that insn, we may
!      delete the insn that actually sets the register if the register dies
!      there and that is incorrect.  */
  
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
      {
--- 1010,1024 ----
  	mark_elimination (ep->from, ep->to);
  
    /* If a pseudo has no hard reg, delete the insns that made the equivalence.
!      We used to recursively delete anything else now dead.  That is incorrect
!      if that insn didn't set the register (i.e., it copied the register to
!      memory); it is also incorrect for a block local equivalence as the memory
!      address may refer to another pseudo which still needs proper
!      initialization. 
! 
!      That code predated running dead code elimination after reload, so rather
!      that trying to identify all the dead code here (and get it wrong), just
!      delete the equivalencing insn and let DCE do its job.  */
  
    for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
      {
*************** reload (rtx first, int global)
*** 1034,1041 ****
  	      if (NOTE_P (equiv_insn)
  		  || can_throw_internal (equiv_insn))
  		;
- 	      else if (reg_set_p (regno_reg_rtx[i], PATTERN (equiv_insn)))
- 		delete_dead_insn (equiv_insn);
  	      else
  		SET_INSN_DELETED (equiv_insn);
  	    }
--- 1037,1042 ----
*************** spill_failure (rtx insn, enum reg_class 
*** 2114,2140 ****
      }
  }
  \f
- /* Delete an unneeded INSN and any previous insns who sole purpose is loading
-    data that is dead in INSN.  */
- 
- static void
- delete_dead_insn (rtx insn)
- {
-   rtx prev = prev_active_insn (insn);
-   rtx prev_dest;
- 
-   /* If the previous insn sets a register that dies in our insn, delete it
-      too.  */
-   if (prev && GET_CODE (PATTERN (prev)) == SET
-       && (prev_dest = SET_DEST (PATTERN (prev)), REG_P (prev_dest))
-       && reg_mentioned_p (prev_dest, PATTERN (insn))
-       && find_regno_note (insn, REG_DEAD, REGNO (prev_dest))
-       && ! side_effects_p (SET_SRC (PATTERN (prev))))
-     delete_dead_insn (prev);
- 
-   SET_INSN_DELETED (insn);
- }
- 
  /* Modify the home of pseudo-reg I.
     The new home is present in reg_renumber[I].
  
--- 2115,2120 ----
*************** eliminate_regs_in_insn (rtx insn, int re
*** 3319,3325 ****
  	       process it since it won't be used unless something changes.  */
  	    if (replace)
  	      {
! 		delete_dead_insn (insn);
  		return 1;
  	      }
  	    val = 1;
--- 3299,3305 ----
  	       process it since it won't be used unless something changes.  */
  	    if (replace)
  	      {
! 		SET_INSN_DELETED (insn);
  		return 1;
  	      }
  	    val = 1;

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RFA PR middle-end/48770
@ 2011-06-14 15:38 Jeff Law
  2011-06-22 15:33 ` Bernd Schmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2011-06-14 15:38 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


This version incorporates suggestions from Bernd.  Basically we have
reload1.c set reload_completed internally rather than deferring it into
ira.c.  That allows the call to reload() to return whether or not a DCE
pass is desirable at the end of reload.

That in turn allows us to avoid the DF clumsiness of the previous version.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.

Bernd is still seeing some differences on mips64-linux; I've been unable
to reproduce those.  Bernd, if you can send me the dump files privately,
I'm more than happy to take a look at any remaining codegen differences
this patch is triggering.

Thanks,
Jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN938LAAoJEBRtltQi2kC7JxIH/jexv1Wx3RZkba8fgBMbrYYg
QLPv273smckcvITNaOdMKSRRbq/8x+hiGI4VClYX3z1tGrlIaDf+n0S/mOGmMDc3
yjxeXRBf0F8QPmkt+QG+Ck6TH3+ya2OOWmP6/RNCBQdaf7ViVuBI+IlGzhEia1OH
YL+3yDTfLpAgJ9BYTpaIB8o9m/cAAx0Rfnwgx9gcQzFGSPgEep1tg+gnxoyMbvGX
IohygwiMkU27JLokeanowL9d2H7L0kYMX1S0biDOdlm1wLI9n3JfLO9PPF0SLv8A
EESCaRmeJRH93wlNLb5qpacESgQOc6B6++zCjf1W22/GVcZIe9WKaOuxKtsoU/I=
=ZEQd
-----END PGP SIGNATURE-----

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 6554 bytes --]

	PR middle-end/48770
	* reload.h (reload): Change to return a bool.
	* ira.c (ira): If requested by reload, run a fast DCE pass after
	reload has completed.  Fix comment typo.
	* reload1.c (need_dce): New file scoped static.
	(reload): Set reload_completed here.  Return whether or not a DCE
	pass after reload is needed.
	(delete_dead_insn): Set need_dce as needed.

	PR middle-end/48770
	* gcc.dg/pr48770.c: New test.

Index: reload.h
===================================================================
*** reload.h	(revision 174696)
--- reload.h	(working copy)
*************** extern void reload_cse_regs (rtx);
*** 420,426 ****
  extern void init_reload (void);
  
  /* The reload pass itself.  */
! extern int reload (rtx, int);
  
  /* Mark the slots in regs_ever_live for the hard regs
     used by pseudo-reg number REGNO.  */
--- 420,426 ----
  extern void init_reload (void);
  
  /* The reload pass itself.  */
! extern bool reload (rtx, int);
  
  /* Mark the slots in regs_ever_live for the hard regs
     used by pseudo-reg number REGNO.  */
Index: testsuite/gcc.dg/pr48770.c
===================================================================
*** testsuite/gcc.dg/pr48770.c	(revision 0)
--- testsuite/gcc.dg/pr48770.c	(revision 0)
***************
*** 0 ****
--- 1,21 ----
+ /* { dg-do run } */
+ /* { dg-options "-O -fprofile-arcs -fPIC -fno-dce -fno-forward-propagate" } */
+ 
+ int test_goto2 (int f)
+ {
+   int i;
+   for (i = 0; ({_Bool a = i < 10;a;}); i++)
+   {
+     if (i == f)
+       goto lab2;
+   }
+   return 4;
+ lab2:
+   return 8;
+ }
+ 
+ int main ()
+ {
+   test_goto2 (30);
+   return 0;
+ }
Index: ira.c
===================================================================
*** ira.c	(revision 174759)
--- ira.c	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 383,388 ****
--- 383,389 ----
  #include "integrate.h"
  #include "ggc.h"
  #include "ira-int.h"
+ #include "dce.h"
  
  
  struct target_ira default_target_ira;
*************** ira (FILE *f)
*** 3526,3531 ****
--- 3527,3533 ----
    int rebuild_p;
    int saved_flag_ira_share_spill_slots;
    basic_block bb;
+   bool need_dce;
  
    timevar_push (TV_IRA);
  
*************** ira (FILE *f)
*** 3717,3723 ****
    df_set_flags (DF_NO_INSN_RESCAN);
    build_insn_chain ();
  
!   reload_completed = !reload (get_insns (), ira_conflicts_p);
  
    timevar_pop (TV_RELOAD);
  
--- 3719,3725 ----
    df_set_flags (DF_NO_INSN_RESCAN);
    build_insn_chain ();
  
!   need_dce = reload (get_insns (), ira_conflicts_p);
  
    timevar_pop (TV_RELOAD);
  
*************** ira (FILE *f)
*** 3760,3766 ****
  #endif
  
    /* The code after the reload has changed so much that at this point
!      we might as well just rescan everything.  Not that
       df_rescan_all_insns is not going to help here because it does not
       touch the artificial uses and defs.  */
    df_finish_pass (true);
--- 3762,3768 ----
  #endif
  
    /* The code after the reload has changed so much that at this point
!      we might as well just rescan everything.  Note that
       df_rescan_all_insns is not going to help here because it does not
       touch the artificial uses and defs.  */
    df_finish_pass (true);
*************** ira (FILE *f)
*** 3772,3777 ****
--- 3774,3782 ----
    if (optimize)
      df_analyze ();
  
+   if (need_dce && optimize)
+     run_fast_dce ();
+ 
    timevar_pop (TV_IRA);
  }
  
Index: reload1.c
===================================================================
*** reload1.c	(revision 174759)
--- reload1.c	(working copy)
*************** static char *reload_insn_firstobj;
*** 250,255 ****
--- 250,259 ----
     examine.  */
  struct insn_chain *reload_insn_chain;
  
+ /* TRUE if we potentially left dead insns in the insn stream and want to
+    run DCE immediately after reload, FALSE otherwise.  */
+ static bool need_dce;
+ 
  /* List of all insns needing reloads.  */
  static struct insn_chain *insns_need_reload;
  \f
*************** static int *temp_pseudo_reg_arr;
*** 695,704 ****
     If GLOBAL is zero, we do not have enough information to do that,
     so any pseudo reg that is spilled must go to the stack.
  
!    Return value is nonzero if reload failed
!    and we must not do any more for this function.  */
  
! int
  reload (rtx first, int global)
  {
    int i, n;
--- 699,709 ----
     If GLOBAL is zero, we do not have enough information to do that,
     so any pseudo reg that is spilled must go to the stack.
  
!    Return value is TRUE if reload likely left dead insns in the
!    stream and a DCE pass should be run to elimiante them.  Else the
!    return value is FALSE.  */
  
! bool
  reload (rtx first, int global)
  {
    int i, n;
*************** reload (rtx first, int global)
*** 1329,1335 ****
  
    gcc_assert (bitmap_empty_p (&spilled_pseudos));
  
!   return failure;
  }
  
  /* Yet another special case.  Unfortunately, reg-stack forces people to
--- 1334,1342 ----
  
    gcc_assert (bitmap_empty_p (&spilled_pseudos));
  
!   reload_completed = !failure;
! 
!   return need_dce;
  }
  
  /* Yet another special case.  Unfortunately, reg-stack forces people to
*************** delete_dead_insn (rtx insn)
*** 2123,2136 ****
    rtx prev = prev_active_insn (insn);
    rtx prev_dest;
  
!   /* If the previous insn sets a register that dies in our insn, delete it
!      too.  */
    if (prev && GET_CODE (PATTERN (prev)) == SET
        && (prev_dest = SET_DEST (PATTERN (prev)), REG_P (prev_dest))
        && reg_mentioned_p (prev_dest, PATTERN (insn))
        && find_regno_note (insn, REG_DEAD, REGNO (prev_dest))
        && ! side_effects_p (SET_SRC (PATTERN (prev))))
!     delete_dead_insn (prev);
  
    SET_INSN_DELETED (insn);
  }
--- 2130,2148 ----
    rtx prev = prev_active_insn (insn);
    rtx prev_dest;
  
!   /* If the previous insn sets a register that dies in our insn make
!      a note that we want to run DCE immediately after reload.
! 
!      We used to delete the previous insn & recurse, but that's wrong for
!      block local equivalences.  Instead of trying to figure out the exact
!      circumstances where we can delete the potentially dead insns, just
!      let DCE do the job.  */
    if (prev && GET_CODE (PATTERN (prev)) == SET
        && (prev_dest = SET_DEST (PATTERN (prev)), REG_P (prev_dest))
        && reg_mentioned_p (prev_dest, PATTERN (insn))
        && find_regno_note (insn, REG_DEAD, REGNO (prev_dest))
        && ! side_effects_p (SET_SRC (PATTERN (prev))))
!     need_dce = 1;
  
    SET_INSN_DELETED (insn);
  }

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-23 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-27 19:10 RFA PR middle-end/48770 Jeff Law
2011-05-30 12:34 ` Bernd Schmidt
2011-05-31 18:33   ` Jeff Law
2011-06-14 15:38 Jeff Law
2011-06-22 15:33 ` Bernd Schmidt
2011-06-23 21:34   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).