Avoid most of calls to side_effects_p
Jan Hubicka
jh@suse.cz
Thu Apr 17 13:05:00 GMT 2003
> Jan Hubicka wrote:
>
> >*************** count_reg_usage (x, counts, dest, incr)
> >*** 7494,7508 ****
> > /* 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);
> >-
> >- /* If SRC has side-effects, then we can't delete this insn, so the
> >- usage of SET_DEST inside SRC counts.
> >-
> >- ??? Strictly-speaking, we might be preserving this insn
> >- because some other SET has side-effects, but that's hard
> >- to do and can't happen now. */
> > count_reg_usage (SET_SRC (x), counts,
> >! side_effects_p (SET_SRC (x)) ? NULL_RTX : SET_DEST
> (x),
> > incr);
> > return;
> >
> >--- 7494,7501 ----
> > /* 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,
> >! SET_DEST (x),
> > incr);
> > return;
>
> This causes miscompiles of the Linux kernel on s390.
>
> The problem is code like this:
>
> unsigned long __xchg(unsigned char * ptr)
> {
> asm volatile ("ptr in %0"
> : "+a" (ptr) : : "0", "1", "2" );
> }
>
> which generates:
>
> (insn 3 2 4 (nil) (set (reg/v/f:DI 41)
> (reg:DI 2 %r2)) -1 (nil)
> (nil))
>
> (insn 9 7 10 (nil) (parallel [
> (set (reg/v/f:DI 41)
> (asm_operands/v:DI ("ptr in %0") ("=a") 0 [
> (reg/v/f:DI 41)
> ]
> [
> (asm_input:DI ("0"))
> ] ("qdio.i") 4))
> (clobber (reg:QI 2 %r2))
> (clobber (reg:QI 1 %r1))
> (clobber (reg:QI 0 %r0))
> ]) -1 (nil)
> (nil))
>
>
> Now, delete_trivially_dead_insns deletes insn 3, because
> count_reg_usage reports 0 usage of reg 41. But of course,
> as insn 9 is a volatile asm, it is not deleted, and the
> use of reg 41 becomes uninitialized ...
Sorry for the delay. I hope this patch will solve your problem.
Without re-introducing those insanely many check this appears to be only
sollution left, even when I don't like idea of passing extra argument to
the already expensive recursive function it appears to make no
measurable slowdowns.
Can you please test whether it helps your testcase?
I am currently testing it on i386.
Honza
Thu Apr 17 14:44:42 CEST 2003 Jan Hubicka <jh@suse.cz>
* cse.c (count_reg_usage): New argument INSN; verify that the insn
can be deleted before not counting register insn uses and modifies.
Index: cse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cse.c,v
retrieving revision 1.259
diff -c -3 -p -r1.259 cse.c
*** cse.c 10 Apr 2003 05:24:26 -0000 1.259
--- cse.c 17 Apr 2003 12:44:26 -0000
*************** static void invalidate_skipped_block PAR
*** 651,657 ****
static void cse_check_loop_start PARAMS ((rtx, rtx, void *));
static void cse_set_around_loop PARAMS ((rtx, rtx, rtx));
static rtx cse_basic_block PARAMS ((rtx, rtx, struct branch_path *, int));
! static void count_reg_usage PARAMS ((rtx, int *, rtx, int));
static int check_for_label_ref PARAMS ((rtx *, void *));
extern void dump_class PARAMS ((struct table_elt*));
static struct cse_reg_info * get_cse_reg_info PARAMS ((unsigned int));
--- 651,657 ----
static void cse_check_loop_start PARAMS ((rtx, rtx, void *));
static void cse_set_around_loop PARAMS ((rtx, rtx, rtx));
static rtx cse_basic_block PARAMS ((rtx, rtx, struct branch_path *, int));
! static void count_reg_usage PARAMS ((rtx, int *, rtx, int, rtx));
static int check_for_label_ref PARAMS ((rtx *, void *));
extern void dump_class PARAMS ((struct table_elt*));
static struct cse_reg_info * get_cse_reg_info PARAMS ((unsigned int));
*************** check_for_label_ref (rtl, data)
*** 7452,7462 ****
modify the liveness of DEST. */
static void
! count_reg_usage (x, counts, dest, incr)
rtx x;
int *counts;
rtx dest;
int incr;
{
enum rtx_code code;
rtx note;
--- 7452,7463 ----
modify the liveness of DEST. */
static void
! count_reg_usage (x, counts, dest, incr, insn)
rtx x;
int *counts;
rtx dest;
int incr;
+ rtx insn;
{
enum rtx_code code;
rtx note;
*************** count_reg_usage (x, counts, dest, incr)
*** 7469,7475 ****
switch (code = GET_CODE (x))
{
case REG:
! if (x != dest)
counts[REGNO (x)] += incr;
return;
--- 7470,7480 ----
switch (code = GET_CODE (x))
{
case REG:
! if (x != dest
! /* Do not delete instructions initializing operands needed by dead
! instruction with side effects. */
! || (!counts[REGNO (x)] && incr > 0
! && insn_live_p (insn, NULL)))
counts[REGNO (x)] += incr;
return;
*************** count_reg_usage (x, counts, dest, incr)
*** 7487,7518 ****
/* 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,
SET_DEST (x),
! incr);
return;
case CALL_INSN:
! count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, NULL_RTX, incr);
/* Fall through. */
case INSN:
case JUMP_INSN:
! count_reg_usage (PATTERN (x), counts, NULL_RTX, incr);
/* Things used in a REG_EQUAL note aren't dead since loop may try to
use them. */
note = find_reg_equal_equiv_note (x);
if (note)
! count_reg_usage (XEXP (note, 0), counts, NULL_RTX, incr);
return;
case INSN_LIST:
--- 7492,7524 ----
/* 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, insn);
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, insn);
count_reg_usage (SET_SRC (x), counts,
SET_DEST (x),
! incr, insn);
return;
case CALL_INSN:
! count_reg_usage (CALL_INSN_FUNCTION_USAGE (x), counts, NULL_RTX, incr,
! insn);
/* Fall through. */
case INSN:
case JUMP_INSN:
! count_reg_usage (PATTERN (x), counts, NULL_RTX, incr, insn);
/* Things used in a REG_EQUAL note aren't dead since loop may try to
use them. */
note = find_reg_equal_equiv_note (x);
if (note)
! count_reg_usage (XEXP (note, 0), counts, NULL_RTX, incr, insn);
return;
case INSN_LIST:
*************** count_reg_usage (x, counts, dest, incr)
*** 7526,7535 ****
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);
}
}
--- 7532,7541 ----
for (i = GET_RTX_LENGTH (code) - 1; i >= 0; i--)
{
if (fmt[i] == 'e')
! count_reg_usage (XEXP (x, i), counts, dest, incr, insn);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
! count_reg_usage (XVECEXP (x, i, j), counts, dest, incr, insn);
}
}
*************** set_live_p (set, insn, counts)
*** 7557,7563 ****
#endif
else if (GET_CODE (SET_DEST (set)) != REG
|| REGNO (SET_DEST (set)) < FIRST_PSEUDO_REGISTER
! || counts[REGNO (SET_DEST (set))] != 0
|| side_effects_p (SET_SRC (set))
/* An ADDRESSOF expression can turn into a use of the
internal arg pointer, so always consider the
--- 7563,7569 ----
#endif
else if (GET_CODE (SET_DEST (set)) != REG
|| REGNO (SET_DEST (set)) < FIRST_PSEUDO_REGISTER
! || (counts && counts[REGNO (SET_DEST (set))] != 0)
|| side_effects_p (SET_SRC (set))
/* An ADDRESSOF expression can turn into a use of the
internal arg pointer, so always consider the
*************** set_live_p (set, insn, counts)
*** 7568,7574 ****
return false;
}
! /* Return true if insn is live. */
static bool
insn_live_p (insn, counts)
--- 7574,7582 ----
return false;
}
! /* Return true if insn is live.
! When COUNTS is NULL return true if insn is live because of the side effects.
! */
static bool
insn_live_p (insn, counts)
*************** dead_libcall_p (insn, counts)
*** 7623,7638 ****
new = XEXP (note, 0);
/* While changing insn, we must update the counts accordingly. */
! count_reg_usage (insn, counts, NULL_RTX, -1);
if (set && 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;
}
--- 7631,7646 ----
new = XEXP (note, 0);
/* While changing insn, we must update the counts accordingly. */
! count_reg_usage (insn, counts, NULL_RTX, -1, insn);
if (set && validate_change (insn, &SET_SRC (set), new, 0))
{
! count_reg_usage (insn, counts, NULL_RTX, 1, insn);
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, insn);
}
return false;
}
*************** delete_trivially_dead_insns (insns, nreg
*** 7659,7665 ****
/* First count the number of times each register is used. */
counts = (int *) 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
{
--- 7667,7673 ----
/* First count the number of times each register is used. */
counts = (int *) xcalloc (nreg, sizeof (int));
for (insn = next_real_insn (insns); insn; insn = next_real_insn (insn))
! count_reg_usage (insn, counts, NULL_RTX, 1, insn);
do
{
*************** delete_trivially_dead_insns (insns, nreg
*** 7703,7709 ****
if (! live_insn)
{
! count_reg_usage (insn, counts, NULL_RTX, -1);
delete_insn_and_edges (insn);
ndead++;
}
--- 7711,7717 ----
if (! live_insn)
{
! count_reg_usage (insn, counts, NULL_RTX, -1, insn);
delete_insn_and_edges (insn);
ndead++;
More information about the Gcc-patches
mailing list