This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH/RFT] Fix PR optimization/11320


Hi,

This is a regression present on the 3.3 branch for IA-64, reported by 
Andreas. The machine-dependent reorg pass swaps two insns:

(jump_insn 82 88 122 3 0x401b54c0 (set (pc)
        (if_then_else (eq (reg:BI 262 p6 [361])
                (const_int 0 [0x0]))
            (label_ref 100)
            (pc))) 214 {*br_true} (insn_list 81 (nil))
    (expr_list:REG_DEAD (reg:BI 262 p6 [361])
        (expr_list:REG_BR_PROB (const_int 5000 [0x1388])
            (nil))))
;; End of basic block 3, registers live:
 12 [r12] 14 [r14] 32 [r35] 33 [r36] 329 [retaddr]

;; Start of basic block 4, registers live: 12 [r12] 14 [r14] 32 [r35] 33 
[r36] 329 [retaddr]
(note 122 82 96 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

(insn 96 122 159 4 0x401b54c0 (set (reg/f:DI 14 r14 [340])
        (lo_sum:DI (reg:DI 14 r14 [365])
            (symbol_ref/f:DI ("*.LC2")))) 12 {*load_symptr_low} (nil)
    (expr_list:REG_EQUAL (symbol_ref/f:DI ("*.LC2"))
        (nil)))

because it doesn't see than insn 96 depends upon insn 82.

However, the dependency is sort of indirect since the target of the JUMP 
doesn't consume r14. Instead, the JUMP is meant to guard insn 96 from using 
a value that has been conditionally written to r14, after a branch was 
merged in the main codepath:

(insn 131 81 88 3 (nil) (cond_exec (eq (reg:BI 262 p6 [361])
            (const_int 0 [0x0]))
        (set (reg:SI 14 r14 [363])
            (const_int 1 [0x1]))) 424 {mf+5} (nil)
    (expr_list:REG_EQUIV (const_int 1 [0x1])
        (nil)))

(insn 88 131 82 3 0x401b54c0 (cond_exec (eq (reg:BI 262 p6 [361])
            (const_int 0 [0x0]))
        (set (mem/f:SI (reg/f:DI 15 r15 [362]) [5 ap_standalone+0 S4 A32])
            (reg:SI 14 r14 [363]))) 424 {mf+5} (insn_list 131 (nil))
    (expr_list:REG_DEAD (reg/f:DI 15 r15 [362])
        (expr_list:REG_EQUAL (const_int 1 [0x1])
            (nil))))

(jump_insn 82 88 122 3 0x401b54c0 (set (pc)
        (if_then_else (eq (reg:BI 262 p6 [361])
                (const_int 0 [0x0]))
            (label_ref 100)
            (pc))) 214 {*br_true} (insn_list 81 (nil))
    (expr_list:REG_DEAD (reg:BI 262 p6 [361])
        (expr_list:REG_BR_PROB (const_int 5000 [0x1388])
            (nil))))


The proposed fix is to say that JUMP_INSNs set the registers that are live on 
entry of the fallthrough block and are conditionally set before the jump, 
because they can be considered as restoring the former value of the 
conditionally set registers. This ensures that insn 96 has a true dependency
upon insn 82 and prevents the swapping.

I know that IA-64 is very sensitive to scheduling because of EPIC, but I 
don't have enough experience to evaluate the effects of the patch and I 
don't have access to Itanium hardware.

Andreas, would you mind bootstrapping/regtesting the patch (against the 3.3 
branch) and maybe comparing the bootstrap times?

-- 
Eric Botcazou


2003-07-10  Eric Botcazou  <ebotcazou@libertysurf.fr>

	PR optimization/11320
	* sched-int.h (struct deps) [reg_conditional_sets]: New field.
	(struct sched_info) [compute_jump_reg_dependencies]: New prototype.
	* sched-deps.c (sched_analyze_insn) [JUMP_INSN]: Update call to
	current_sched_info->compute_jump_reg_dependencies. Record which
	registers are used and which registers are set by the jump.
	Clear deps->reg_conditional_sets after a barrier.
	Set deps->reg_conditional_sets if the insn is a COND_EXEC.
	Clear deps->reg_conditional_sets if the insn is not a COND_EXEC.
	(init_deps): Initialize reg_conditional_sets.
	(free_deps): Clear reg_conditional_sets.
	* sched-ebb.c (compute_jump_reg_dependencies): New prototype.
	Mark registers live on entry of the fallthrough block and conditionally
	set as set by the jump. Mark registers live on entry of non-fallthrough
	blocks as used by the jump.
	* sched-rgn.c (compute_jump_reg_dependencies): New prototype.
	Mark new parameters as unused.


2003-07-10  Eric Botcazou  <ebotcazou@libertysurf.fr>

	* gcc.c-torture/execute/20030710-1.c: New test.
Index: sched-deps.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/sched-deps.c,v
retrieving revision 1.49
diff -c -p -r1.49 sched-deps.c
*** sched-deps.c	20 Nov 2002 10:03:34 -0000	1.49
--- sched-deps.c	10 Jul 2003 12:43:42 -0000
*************** sched_analyze_insn (deps, x, insn, loop_
*** 977,988 ****
        else
  	{
  	  rtx pending, pending_mem;
! 	  regset_head tmp;
! 	  INIT_REG_SET (&tmp);
  
! 	  (*current_sched_info->compute_jump_reg_dependencies) (insn, &tmp);
! 	  IOR_REG_SET (reg_pending_uses, &tmp);
! 	  CLEAR_REG_SET (&tmp);
  
  	  /* All memory writes and volatile reads must happen before the
  	     jump.  Non-volatile reads must happen before the jump iff
--- 977,993 ----
        else
  	{
  	  rtx pending, pending_mem;
! 	  regset_head tmp_uses, tmp_sets;
! 	  INIT_REG_SET (&tmp_uses);
! 	  INIT_REG_SET (&tmp_sets);
! 
! 	  (*current_sched_info->compute_jump_reg_dependencies)
! 	    (insn, &deps->reg_conditional_sets, &tmp_uses, &tmp_sets);
! 	  IOR_REG_SET (reg_pending_uses, &tmp_uses);
! 	  IOR_REG_SET (reg_pending_sets, &tmp_sets);
  
! 	  CLEAR_REG_SET (&tmp_uses);
! 	  CLEAR_REG_SET (&tmp_sets);
  
  	  /* All memory writes and volatile reads must happen before the
  	     jump.  Non-volatile reads must happen before the jump iff
*************** sched_analyze_insn (deps, x, insn, loop_
*** 1079,1084 ****
--- 1084,1090 ----
  	}
  
        flush_pending_lists (deps, insn, true, true);
+       CLEAR_REG_SET (&deps->reg_conditional_sets);
        reg_pending_barrier = false;
      }
    else
*************** sched_analyze_insn (deps, x, insn, loop_
*** 1110,1115 ****
--- 1116,1122 ----
  	      add_dependence_list (insn, reg_last->clobbers, REG_DEP_OUTPUT);
  	      add_dependence_list (insn, reg_last->uses, REG_DEP_ANTI);
  	      reg_last->sets = alloc_INSN_LIST (insn, reg_last->sets);
+ 	      SET_REGNO_REG_SET (&deps->reg_conditional_sets, i);
  	    });
  	}
        else
*************** sched_analyze_insn (deps, x, insn, loop_
*** 1158,1163 ****
--- 1165,1171 ----
  	      reg_last->sets = alloc_INSN_LIST (insn, reg_last->sets);
  	      reg_last->uses_length = 0;
  	      reg_last->clobbers_length = 0;
+ 	      CLEAR_REGNO_REG_SET (&deps->reg_conditional_sets, i);
  	    });
  	}
  
*************** init_deps (deps)
*** 1484,1489 ****
--- 1492,1498 ----
    deps->reg_last = (struct deps_reg *)
      xcalloc (max_reg, sizeof (struct deps_reg));
    INIT_REG_SET (&deps->reg_last_in_use);
+   INIT_REG_SET (&deps->reg_conditional_sets);
  
    deps->pending_read_insns = 0;
    deps->pending_read_mems = 0;
*************** free_deps (deps)
*** 1526,1531 ****
--- 1535,1541 ----
  	free_INSN_LIST_list (&reg_last->clobbers);
      });
    CLEAR_REG_SET (&deps->reg_last_in_use);
+   CLEAR_REG_SET (&deps->reg_conditional_sets);
  
    free (deps->reg_last);
  }
Index: sched-ebb.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/sched-ebb.c,v
retrieving revision 1.17
diff -c -p -r1.17 sched-ebb.c
*** sched-ebb.c	17 Jun 2002 17:16:37 -0000	1.17
--- sched-ebb.c	10 Jul 2003 12:43:46 -0000
*************** static int schedule_more_p PARAMS ((void
*** 52,58 ****
  static const char *ebb_print_insn PARAMS ((rtx, int));
  static int rank PARAMS ((rtx, rtx));
  static int contributes_to_priority PARAMS ((rtx, rtx));
! static void compute_jump_reg_dependencies PARAMS ((rtx, regset));
  static void schedule_ebb PARAMS ((rtx, rtx));
  
  /* Return nonzero if there are more insns that should be scheduled.  */
--- 52,59 ----
  static const char *ebb_print_insn PARAMS ((rtx, int));
  static int rank PARAMS ((rtx, rtx));
  static int contributes_to_priority PARAMS ((rtx, rtx));
! static void compute_jump_reg_dependencies PARAMS ((rtx, regset, regset,
! 						   regset));
  static void schedule_ebb PARAMS ((rtx, rtx));
  
  /* Return nonzero if there are more insns that should be scheduled.  */
*************** contributes_to_priority (next, insn)
*** 160,181 ****
    return 1;
  }
  
! /* INSN is a JUMP_INSN.  Store the set of registers that must be considered
!    to be set by this jump in SET.  */
  
  static void
! compute_jump_reg_dependencies (insn, set)
       rtx insn;
!      regset set;
  {
    basic_block b = BLOCK_FOR_INSN (insn);
    edge e;
    for (e = b->succ; e; e = e->succ_next)
!     if ((e->flags & EDGE_FALLTHRU) == 0)
!       {
! 	bitmap_operation (set, set, e->dest->global_live_at_start,
! 			  BITMAP_IOR);
!       }
  }
  
  /* Used in schedule_insns to initialize current_sched_info for scheduling
--- 161,190 ----
    return 1;
  }
  
! /* INSN is a JUMP_INSN, COND_SET is the set of registers that are
!    conditionally set before INSN.  Store the set of registers that
!    must be considered as used by this jump in USED and that of
!    registers that must be considered as set in SET.  */
  
  static void
! compute_jump_reg_dependencies (insn, cond_set, used, set)
       rtx insn;
!      regset cond_set, used, set;
  {
    basic_block b = BLOCK_FOR_INSN (insn);
    edge e;
    for (e = b->succ; e; e = e->succ_next)
!     if (e->flags & EDGE_FALLTHRU)
!       /* The jump may be a by-product of a branch that has been merged
! 	 in the main codepath after being conditionalized.  Therefore
! 	 it may guard the fallthrough block from using a value that has
! 	 conditionally overwritten that of the main codepath.  So we
! 	 consider that it restores the value of the main codepath.  */
!       bitmap_operation (set, e->dest->global_live_at_start, cond_set,
! 			BITMAP_AND);
!     else
!       bitmap_operation (used, used, e->dest->global_live_at_start,
! 			BITMAP_IOR);
  }
  
  /* Used in schedule_insns to initialize current_sched_info for scheduling
Index: sched-int.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/sched-int.h,v
retrieving revision 1.23
diff -c -p -r1.23 sched-int.h
*** sched-int.h	27 Sep 2002 12:48:02 -0000	1.23
--- sched-int.h	10 Jul 2003 12:43:46 -0000
*************** struct deps
*** 112,117 ****
--- 112,120 ----
    /* Element N is set for each register that has any nonzero element
       in reg_last[N].{uses,sets,clobbers}.  */
    regset_head reg_last_in_use;
+ 
+   /* Element N is set for each register that is conditionally set.  */
+   regset_head reg_conditional_sets;
  };
  
  /* This structure holds some state of the current scheduling pass, and
*************** struct sched_info
*** 146,154 ****
       calculations.  */
    int (*contributes_to_priority) PARAMS ((rtx, rtx));
    /* Called when computing dependencies for a JUMP_INSN.  This function
!      should store the set of registers that must be considered as set by
!      the jump in the regset.  */
!   void (*compute_jump_reg_dependencies) PARAMS ((rtx, regset));
  
    /* The boundaries of the set of insns to be scheduled.  */
    rtx prev_head, next_tail;
--- 149,157 ----
       calculations.  */
    int (*contributes_to_priority) PARAMS ((rtx, rtx));
    /* Called when computing dependencies for a JUMP_INSN.  This function
!      should store the set of registers that must be considered as used
!      and the set of registers that must be considered as set by the jump.  */
!   void (*compute_jump_reg_dependencies) PARAMS ((rtx, regset, regset, regset));
  
    /* The boundaries of the set of insns to be scheduled.  */
    rtx prev_head, next_tail;
Index: sched-rgn.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/sched-rgn.c,v
retrieving revision 1.50
diff -c -p -r1.50 sched-rgn.c
*** sched-rgn.c	13 Dec 2002 00:17:21 -0000	1.50
--- sched-rgn.c	10 Jul 2003 12:43:54 -0000
*************** static int schedule_more_p PARAMS ((void
*** 1967,1973 ****
  static const char *rgn_print_insn PARAMS ((rtx, int));
  static int rgn_rank PARAMS ((rtx, rtx));
  static int contributes_to_priority PARAMS ((rtx, rtx));
! static void compute_jump_reg_dependencies PARAMS ((rtx, regset));
  
  /* Return nonzero if there are more insns that should be scheduled.  */
  
--- 1967,1974 ----
  static const char *rgn_print_insn PARAMS ((rtx, int));
  static int rgn_rank PARAMS ((rtx, rtx));
  static int contributes_to_priority PARAMS ((rtx, rtx));
! static void compute_jump_reg_dependencies PARAMS ((rtx, regset, regset,
! 						   regset));
  
  /* Return nonzero if there are more insns that should be scheduled.  */
  
*************** contributes_to_priority (next, insn)
*** 2254,2265 ****
    return BLOCK_NUM (next) == BLOCK_NUM (insn);
  }
  
! /* INSN is a JUMP_INSN.  Store the set of registers that must be considered
!    to be set by this jump in SET.  */
  
  static void
! compute_jump_reg_dependencies (insn, set)
       rtx insn ATTRIBUTE_UNUSED;
       regset set ATTRIBUTE_UNUSED;
  {
    /* Nothing to do here, since we postprocess jumps in
--- 2255,2270 ----
    return BLOCK_NUM (next) == BLOCK_NUM (insn);
  }
  
! /* INSN is a JUMP_INSN, COND_SET is the set of registers that are
!    conditionally set before INSN.  Store the set of registers that
!    must be considered as used by this jump in USED and that of
!    registers that must be considered as set in SET.  */
  
  static void
! compute_jump_reg_dependencies (insn, cond_set, used, set)
       rtx insn ATTRIBUTE_UNUSED;
+      regset cond_set ATTRIBUTE_UNUSED;
+      regset used ATTRIBUTE_UNUSED;
       regset set ATTRIBUTE_UNUSED;
  {
    /* Nothing to do here, since we postprocess jumps in
/* PR optimization/11320 */
/* Origin: Andreas Schwab <schwab@suse.de> */

/* Verify that the scheduler correctly computes the dependencies
   in the presence of conditional instructions.  */

int strcmp (const char *, const char *);
int ap_standalone;

const char *ap_check_cmd_context (void *a, int b)
{
  return 0;
}

const char *server_type (void *a, void *b, char *arg)
{
  const char *err = ap_check_cmd_context (a, 0x01|0x02|0x04|0x08|0x10);
  if (err)
    return err;

  if (!strcmp (arg, "inetd"))
    ap_standalone = 0;
  else if (!strcmp (arg, "standalone"))
      ap_standalone = 1;
  else
    return "ServerType must be either 'inetd' or 'standalone'";

  return 0;
}

int main ()
{
  server_type (0, 0, "standalone");
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]