This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Improve funcorder.c optimisation on ppc-darwin
- From: Joern Rennecke <joern dot rennecke at superh dot com>
- To: gkeating at apple dot com (Geoffrey Keating)
- Cc: gcc-patches at gcc dot gnu dot org, dave dot anglin at nrc-cnrc dot gc dot ca (John David Anglin)
- Date: Mon, 17 May 2004 19:12:48 +0100 (BST)
- Subject: Re: Improve funcorder.c optimisation on ppc-darwin
> This patch:
>
> +2003-10-08 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
> +
> + PR optimization/12142
> + * cse.c (count_reg_usage): In a SET with a REG SET_DEST, count the
> + uses of the register in the SET_SRC. Remove unnecessary argument.
> + * pa.c (legitimize_pic_address): Before reload, use a scratch register
> + for the intermediate result in loading the address of a SYMBOL_REF.
> + Set the MEM_NOTRAP_P flag for the MEM. Add a REG_EQUAL to the insn
> + which loads the SYMBOL_REF address.
>
> caused
> FAIL: gcc.dg/funcorder.c scan-assembler-not link_error
>
> on powerpc-darwin. I can't convince myself that the Dave's patch is either
> correct or incorrect; certainly, it seems like the patch suppresses at
> least some legitimate optimisations. However, I can be sure that it'd
> be better if the Darwin backend used a scratch register in this case,
> so I am making this change.
I've just come across this de-optimization while merging a patch for
a life info problem (delete_trivially_dead_insn can do some deletions that
update_life_info can't, and the latter barfs when doing a local update
and finding global information to be stale).
On processors where it is common that you do a multi-insn sequence to set
a register, not allowing dead multi-insn setter sequences to be deleted
seems rather worrying.
And the PR actually mentioned a much simpler approach that doesn't
cause such an optimization regression.
I have appended a patch to implement this. Most of it is actually backing
out the old patch. I'm testing this on i686-pc-linux-gnu native and
X sh-elf, in the mailine from the 12th May because that was the last time
mainline build for sh-elf without scheduler patches.
2004-05-17 J"orn Rennecke <joern.rennecke@superh.com>
Back out this patch:
2003-10-08 John David Anglin <dave.anglin@nrc-cnrc.gc.ca>
PR optimization/12142
* cse.c (count_reg_usage): In a SET with a REG SET_DEST, count the
uses of the register in the SET_SRC. Remove unnecessary argument.
Replace it with this:
* cse.c (count_reg_usage): In INSN, JUMP_INSN and CALL_INSN cases,
if flag_non_call_exceptions is set and the insn may trap, pass
pc_rtx as dest for recursion.
In SET_SRC part of SET case, if dest is already set, pass it down
unchanged.
Index: cse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cse.c,v
retrieving revision 1.299
diff -p -r1.299 cse.c
*** cse.c 29 Apr 2004 07:50:54 -0000 1.299
--- cse.c 17 May 2004 17:44:12 -0000
*************** static void invalidate_skipped_block (rt
*** 650,656 ****
static void cse_check_loop_start (rtx, rtx, void *);
static void cse_set_around_loop (rtx, rtx, rtx);
static rtx cse_basic_block (rtx, rtx, struct branch_path *, int);
! static void count_reg_usage (rtx, int *, int);
static int check_for_label_ref (rtx *, void *);
extern void dump_class (struct table_elt*);
static struct cse_reg_info * get_cse_reg_info (unsigned int);
--- 650,656 ----
static void cse_check_loop_start (rtx, rtx, void *);
static void cse_set_around_loop (rtx, rtx, rtx);
static rtx cse_basic_block (rtx, rtx, struct branch_path *, int);
! static void count_reg_usage (rtx, int *, rtx, int);
static int check_for_label_ref (rtx *, void *);
extern void dump_class (struct table_elt*);
static struct cse_reg_info * get_cse_reg_info (unsigned int);
*************** check_for_label_ref (rtx *rtl, void *dat
*** 7272,7281 ****
/* Count the number of times registers are used (not set) in X.
COUNTS is an array in which we accumulate the count, INCR is how much
! we count each register usage. */
static void
! count_reg_usage (rtx x, int *counts, int incr)
{
enum rtx_code code;
rtx note;
--- 7272,7287 ----
/* Count the number of times registers are used (not set) in X.
COUNTS is an array in which we accumulate the count, INCR is how much
! we count each register usage.
!
! Don't count a usage of DEST, which is the SET_DEST of a SET which
! contains X in its SET_SRC. This is because such a SET does not
! modify the liveness of DEST.
! DEST is set to pc_rtx for a trapping insn, which means that we must count
! uses of a SET_DEST regardless because the insn can't be deleted here. */
static void
! count_reg_usage (rtx x, int *counts, rtx dest, int incr)
{
enum rtx_code code;
rtx note;
*************** count_reg_usage (rtx x, int *counts, int
*** 7288,7294 ****
switch (code = GET_CODE (x))
{
case REG:
! counts[REGNO (x)] += incr;
return;
case PC:
--- 7294,7301 ----
switch (code = GET_CODE (x))
{
case REG:
! if (x != dest)
! counts[REGNO (x)] += incr;
return;
case PC:
*************** count_reg_usage (rtx x, int *counts, int
*** 7305,7327 ****
/* If we are clobbering a MEM, mark any registers inside the address
as being used. */
if (GET_CODE (XEXP (x, 0)) == MEM)
! count_reg_usage (XEXP (XEXP (x, 0), 0), counts, incr);
return;
case SET:
/* Unless we are setting a REG, count everything in SET_DEST. */
if (GET_CODE (SET_DEST (x)) != REG)
! count_reg_usage (SET_DEST (x), counts, incr);
! count_reg_usage (SET_SRC (x), counts, incr);
return;
case CALL_INSN:
- count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, incr);
- /* Fall through. */
-
case INSN:
case JUMP_INSN:
! count_reg_usage (PATTERN (x), counts, incr);
/* Things used in a REG_EQUAL note aren't dead since loop may try to
use them. */
--- 7312,7339 ----
/* If we are clobbering a MEM, mark any registers inside the address
as being used. */
if (GET_CODE (XEXP (x, 0)) == MEM)
! count_reg_usage (XEXP (XEXP (x, 0), 0), counts, NULL_RTX, incr);
return;
case SET:
/* Unless we are setting a REG, count everything in SET_DEST. */
if (GET_CODE (SET_DEST (x)) != REG)
! count_reg_usage (SET_DEST (x), counts, NULL_RTX, incr);
! count_reg_usage (SET_SRC (x), counts,
! dest ? dest : SET_DEST (x),
! incr);
return;
case CALL_INSN:
case INSN:
case JUMP_INSN:
! /* We expect dest to be NULL_RTX here. If the insn may trap, mark
! this fact by setting DEST to pc_rtx. */
! if (flag_non_call_exceptions && may_trap_p (PATTERN (x)))
! dest = pc_rtx;
! if (code == CALL_INSN)
! count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, dest, incr);
! count_reg_usage (PATTERN (x), counts, dest, incr);
/* Things used in a REG_EQUAL note aren't dead since loop may try to
use them. */
*************** count_reg_usage (rtx x, int *counts, int
*** 7336,7347 ****
Process all the arguments. */
do
{
! count_reg_usage (XEXP (eqv, 0), counts, incr);
eqv = XEXP (eqv, 1);
}
while (eqv && GET_CODE (eqv) == EXPR_LIST);
else
! count_reg_usage (eqv, counts, incr);
}
return;
--- 7348,7359 ----
Process all the arguments. */
do
{
! count_reg_usage (XEXP (eqv, 0), counts, dest, incr);
eqv = XEXP (eqv, 1);
}
while (eqv && GET_CODE (eqv) == EXPR_LIST);
else
! count_reg_usage (eqv, counts, dest, incr);
}
return;
*************** count_reg_usage (rtx x, int *counts, int
*** 7351,7365 ****
/* FUNCTION_USAGE expression lists may include (CLOBBER (mem /u)),
involving registers in the address. */
|| GET_CODE (XEXP (x, 0)) == CLOBBER)
! count_reg_usage (XEXP (x, 0), counts, incr);
! count_reg_usage (XEXP (x, 1), counts, incr);
return;
case ASM_OPERANDS:
/* Iterate over just the inputs, not the constraints as well. */
for (i = ASM_OPERANDS_INPUT_LENGTH (x) - 1; i >= 0; i--)
! count_reg_usage (ASM_OPERANDS_INPUT (x, i), counts, incr);
return;
case INSN_LIST:
--- 7363,7381 ----
/* FUNCTION_USAGE expression lists may include (CLOBBER (mem /u)),
involving registers in the address. */
|| GET_CODE (XEXP (x, 0)) == CLOBBER)
! count_reg_usage (XEXP (x, 0), counts, NULL_RTX, incr);
! count_reg_usage (XEXP (x, 1), counts, NULL_RTX, incr);
return;
case ASM_OPERANDS:
+ /* If the asm is volatile, then this insn cannot be deleted,
+ and so the inputs *must* be live. */
+ if (MEM_VOLATILE_P (x))
+ dest = NULL_RTX;
/* Iterate over just the inputs, not the constraints as well. */
for (i = ASM_OPERANDS_INPUT_LENGTH (x) - 1; i >= 0; i--)
! count_reg_usage (ASM_OPERANDS_INPUT (x, i), counts, dest, incr);
return;
case INSN_LIST:
*************** count_reg_usage (rtx x, int *counts, int
*** 7373,7382 ****
for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
{
if (fmt[i] == 'e')
! count_reg_usage (XEXP (x, i), counts, incr);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
! count_reg_usage (XVECEXP (x, i, j), counts, incr);
}
}
--- 7389,7398 ----
for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
{
if (fmt[i] == 'e')
! count_reg_usage (XEXP (x, i), counts, dest, incr);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
! count_reg_usage (XVECEXP (x, i, j), counts, dest, incr);
}
}
*************** dead_libcall_p (rtx insn, int *counts)
*** 7468,7478 ****
new = XEXP (note, 0);
/* While changing insn, we must update the counts accordingly. */
! count_reg_usage (insn, counts, -1);
if (validate_change (insn, &SET_SRC (set), new, 0))
{
! count_reg_usage (insn, counts, 1);
remove_note (insn, find_reg_note (insn, REG_RETVAL, NULL_RTX));
remove_note (insn, note);
return true;
--- 7484,7494 ----
new = XEXP (note, 0);
/* While changing insn, we must update the counts accordingly. */
! count_reg_usage (insn, counts, NULL_RTX, -1);
if (validate_change (insn, &SET_SRC (set), new, 0))
{
! count_reg_usage (insn, counts, NULL_RTX, 1);
remove_note (insn, find_reg_note (insn, REG_RETVAL, NULL_RTX));
remove_note (insn, note);
return true;
*************** dead_libcall_p (rtx insn, int *counts)
*** 7483,7496 ****
new = force_const_mem (GET_MODE (SET_DEST (set)), new);
if (new && validate_change (insn, &SET_SRC (set), new, 0))
{
! count_reg_usage (insn, counts, 1);
remove_note (insn, find_reg_note (insn, REG_RETVAL, NULL_RTX));
remove_note (insn, note);
return true;
}
}
! count_reg_usage (insn, counts, 1);
return false;
}
--- 7499,7512 ----
new = force_const_mem (GET_MODE (SET_DEST (set)), new);
if (new && validate_change (insn, &SET_SRC (set), new, 0))
{
! count_reg_usage (insn, counts, NULL_RTX, 1);
remove_note (insn, find_reg_note (insn, REG_RETVAL, NULL_RTX));
remove_note (insn, note);
return true;
}
}
! count_reg_usage (insn, counts, NULL_RTX, 1);
return false;
}
*************** delete_trivially_dead_insns (rtx insns,
*** 7514,7520 ****
/* First count the number of times each register is used. */
counts = xcalloc (nreg, sizeof (int));
for (insn = next_real_insn (insns); insn; insn = next_real_insn (insn))
! count_reg_usage (insn, counts, 1);
do
{
--- 7530,7536 ----
/* First count the number of times each register is used. */
counts = xcalloc (nreg, sizeof (int));
for (insn = next_real_insn (insns); insn; insn = next_real_insn (insn))
! count_reg_usage (insn, counts, NULL_RTX, 1);
do
{
*************** delete_trivially_dead_insns (rtx insns,
*** 7558,7564 ****
if (! live_insn)
{
! count_reg_usage (insn, counts, -1);
delete_insn_and_edges (insn);
ndead++;
}
--- 7574,7580 ----
if (! live_insn)
{
! count_reg_usage (insn, counts, NULL_RTX, -1);
delete_insn_and_edges (insn);
ndead++;
}