[PATCH] PR opt/12280: Avoid RTL sharing in noce_emit_cmove
Jan Hubicka
hubicka@ucw.cz
Sun Nov 9 20:55:00 GMT 2003
> On Sun, Nov 09, 2003 at 04:24:11PM +0100, Jan Hubicka wrote:
> > + case MEM:
> > + /* A MEM is allowed to be shared if its address is constant. */
> > + if (CONSTANT_ADDRESS_P (XEXP (x, 0))
> > + || reload_completed || reload_in_progress)
> > + return;
>
> This is now wrong; it changed recently. Which brings the point
> that there should be exactly one place this logic is maintained.
> I suggest splitting out a predicate that this and copy_rtx and
> so forth can use.
>
Hi,
here is updated patch.
I hope we won't get too many other RTL sharing positives in other parts
of the compiler. Thinking about it, there is not much of special about
ifcvt some other parts don't do.
Perhaps we can think of modifying emit_insn_* to automatically call
unshare_rtl_chain when not emiting into sequence? THat way we can try to
manage instruction chain to have the flag set, but it is not too easy
either. (we need to update other places that substitute RTL, but most of
these do so via validate_change)
Honza
2003-11-09 Jan Hubicka <jh@suse.cz>
* emit-rtl.c (set_used_flags): New.
(rtx_can_be_shared_p): Break out of ....
(copy_rtx_if_shared): ... here.
(check_rtx_sharing, check_rtl_sharing): New.
(unshare_all_rtl_1): Rename to....
(unshare_all_rtl_in_chain): ... this one; make static.
(copy_rtx_if_shared): LABEL_REF chan be shared.
* ifcvt.c (unshare_ifcvt_sequence): New.
(noce_try_move, noce_try_store_flag, noce_try_store_flag_constants,
noce_try_addcc, noce_try_addcc, noce_try_store_flag_mask,
noce_try_cmove, noce_try_store_flag_mask, noce_try_minmax,
noce_try_abs, noce_process_if_block, find_cond_trap
* rtl.h (check_rtl_sharing, set_used_flags, unshare_all_rtl_in_chain):
Declare.
* toplev.c (rest_of_compilation): Check for RTL sharing.
Index: emit-rtl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/emit-rtl.c,v
retrieving revision 1.352
diff -c -3 -p -r1.352 emit-rtl.c
*** emit-rtl.c 4 Nov 2003 09:14:18 -0000 1.352
--- emit-rtl.c 9 Nov 2003 20:30:59 -0000
*************** static rtx make_jump_insn_raw (rtx);
*** 180,186 ****
static rtx make_call_insn_raw (rtx);
static rtx find_line_note (rtx);
static rtx change_address_1 (rtx, enum machine_mode, rtx, int);
- static void unshare_all_rtl_1 (rtx);
static void unshare_all_decls (tree);
static void reset_used_decls (tree);
static void mark_label_nuses (rtx);
--- 180,185 ----
*************** unshare_all_rtl (tree fndecl, rtx insn)
*** 2448,2454 ****
unshare_all_decls (DECL_INITIAL (fndecl));
/* Unshare just about everything else. */
! unshare_all_rtl_1 (insn);
/* Make sure the addresses of stack slots found outside the insn chain
(such as, in DECL_RTL of a variable) are not shared
--- 2447,2453 ----
unshare_all_decls (DECL_INITIAL (fndecl));
/* Unshare just about everything else. */
! unshare_all_rtl_in_chain (insn);
/* Make sure the addresses of stack slots found outside the insn chain
(such as, in DECL_RTL of a variable) are not shared
*************** unshare_all_rtl_again (rtx insn)
*** 2490,2500 ****
unshare_all_rtl (cfun->decl, insn);
}
/* Go through all the RTL insn bodies and copy any invalid shared structure.
Assumes the mark bits are cleared at entry. */
! static void
! unshare_all_rtl_1 (rtx insn)
{
for (; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
--- 2489,2633 ----
unshare_all_rtl (cfun->decl, insn);
}
+ /* Return true when X can be shared. */
+ static bool
+ rtx_can_be_shared_p (rtx x)
+ {
+ switch (GET_CODE (x))
+ {
+ case REG:
+ case QUEUED:
+ case CONST_INT:
+ case CONST_DOUBLE:
+ case CONST_VECTOR:
+ case SYMBOL_REF:
+ case LABEL_REF:
+ case CODE_LABEL:
+ case PC:
+ case CC0:
+ case SCRATCH:
+ /* SCRATCH must be shared because they represent distinct values. */
+ return true;
+
+ case CONST:
+ /* CONST can be shared if it contains a SYMBOL_REF. If it contains
+ a LABEL_REF, it isn't sharable. */
+ if (GET_CODE (XEXP (x, 0)) == PLUS
+ && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF
+ && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT)
+ return true;
+ break;
+
+ case INSN:
+ case JUMP_INSN:
+ case CALL_INSN:
+ case NOTE:
+ case BARRIER:
+ /* The chain of insns is not being copied. */
+ return true;
+
+ default:
+ break;
+ }
+ return false;
+ }
+
+ /* Check that ORIG is not marked when it should not be and mark ORIG as in use,
+ Recursively does the same for subexpressions. */
+
+ static void
+ check_rtx_sharing (rtx orig, rtx insn)
+ {
+ rtx x = orig;
+ int i;
+ enum rtx_code code;
+ const char *format_ptr;
+
+ if (x == 0)
+ return;
+
+ if (rtx_can_be_shared_p (x))
+ return;
+
+ code = GET_CODE (x);
+
+ /* This rtx may not be shared. If it has already been seen,
+ replace it with a copy of itself. */
+
+ if (RTX_FLAG (x, used))
+ {
+ error ("Invalid rtl sharing found in the insn");
+ debug_rtx (insn);
+ error ("Shared rtx");
+ debug_rtx (x);
+ abort ();
+ }
+ RTX_FLAG (x, used) = 1;
+
+ /* Now scan the subexpressions recursively. */
+
+ format_ptr = GET_RTX_FORMAT (code);
+
+ for (i = 0; i < GET_RTX_LENGTH (code); i++)
+ {
+ switch (*format_ptr++)
+ {
+ case 'e':
+ check_rtx_sharing (XEXP (x, i), insn);
+ break;
+
+ case 'E':
+ if (XVEC (x, i) != NULL)
+ {
+ int j;
+ int len = XVECLEN (x, i);
+
+ for (j = 0; j < len; j++)
+ {
+ /* We allow sharing of ASM_OPERANDS inside single instruction. */
+ if (j && GET_CODE (XVECEXP (x, i, j)) == SET
+ && GET_CODE (SET_SRC (XVECEXP (x, i, j))) == ASM_OPERANDS)
+ check_rtx_sharing (SET_DEST (XVECEXP (x, i, j)), insn);
+ else
+ check_rtx_sharing (XVECEXP (x, i, j), insn);
+ }
+ }
+ break;
+ }
+ }
+ return;
+ }
+
+ /* Go through all the RTL insn bodies and chec that there is no inexpected
+ sharing in between the subexpressions. */
+
+ void
+ check_rtl_sharing (void)
+ {
+ rtx p;
+
+ for (p = get_insns (); p; p = NEXT_INSN (p))
+ if (INSN_P (p))
+ {
+ reset_used_flags (PATTERN (p));
+ reset_used_flags (REG_NOTES (p));
+ reset_used_flags (LOG_LINKS (p));
+ }
+
+ for (p = get_insns (); p; p = NEXT_INSN (p))
+ if (INSN_P (p))
+ {
+ check_rtx_sharing (PATTERN (p), p);
+ check_rtx_sharing (REG_NOTES (p), p);
+ check_rtx_sharing (LOG_LINKS (p), p);
+ }
+ }
+
/* Go through all the RTL insn bodies and copy any invalid shared structure.
Assumes the mark bits are cleared at entry. */
! void
! unshare_all_rtl_in_chain (rtx insn)
{
for (; insn; insn = NEXT_INSN (insn))
if (INSN_P (insn))
*************** copy_rtx_if_shared (rtx orig)
*** 2654,2698 ****
if (x == 0)
return 0;
! code = GET_CODE (x);
!
! /* These types may be freely shared. */
!
! switch (code)
! {
! case REG:
! case QUEUED:
! case CONST_INT:
! case CONST_DOUBLE:
! case CONST_VECTOR:
! case SYMBOL_REF:
! case CODE_LABEL:
! case PC:
! case CC0:
! case SCRATCH:
! /* SCRATCH must be shared because they represent distinct values. */
! return x;
!
! case CONST:
! /* CONST can be shared if it contains a SYMBOL_REF. If it contains
! a LABEL_REF, it isn't sharable. */
! if (GET_CODE (XEXP (x, 0)) == PLUS
! && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF
! && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT)
! return x;
! break;
!
! case INSN:
! case JUMP_INSN:
! case CALL_INSN:
! case NOTE:
! case BARRIER:
! /* The chain of insns is not being copied. */
! return x;
! default:
! break;
! }
/* This rtx may not be shared. If it has already been seen,
replace it with a copy of itself. */
--- 2787,2796 ----
if (x == 0)
return 0;
! if (rtx_can_be_shared_p (x))
! return x;
! code = GET_CODE (x);
/* This rtx may not be shared. If it has already been seen,
replace it with a copy of itself. */
*************** reset_used_flags (rtx x)
*** 2798,2803 ****
--- 2896,2964 ----
case 'E':
for (j = 0; j < XVECLEN (x, i); j++)
reset_used_flags (XVECEXP (x, i, j));
+ break;
+ }
+ }
+ }
+
+ /* Set all the USED bits in X to allow copy_rtx_if_shared to be used
+ to look for shared sub-parts. */
+
+ void
+ set_used_flags (rtx x)
+ {
+ int i, j;
+ enum rtx_code code;
+ const char *format_ptr;
+
+ if (x == 0)
+ return;
+
+ code = GET_CODE (x);
+
+ /* These types may be freely shared so we needn't do any resetting
+ for them. */
+
+ switch (code)
+ {
+ case REG:
+ case QUEUED:
+ case CONST_INT:
+ case CONST_DOUBLE:
+ case CONST_VECTOR:
+ case SYMBOL_REF:
+ case CODE_LABEL:
+ case PC:
+ case CC0:
+ return;
+
+ case INSN:
+ case JUMP_INSN:
+ case CALL_INSN:
+ case NOTE:
+ case LABEL_REF:
+ case BARRIER:
+ /* The chain of insns is not being copied. */
+ return;
+
+ default:
+ break;
+ }
+
+ RTX_FLAG (x, used) = 1;
+
+ format_ptr = GET_RTX_FORMAT (code);
+ for (i = 0; i < GET_RTX_LENGTH (code); i++)
+ {
+ switch (*format_ptr++)
+ {
+ case 'e':
+ set_used_flags (XEXP (x, i));
+ break;
+
+ case 'E':
+ for (j = 0; j < XVECLEN (x, i); j++)
+ set_used_flags (XVECEXP (x, i, j));
break;
}
}
Index: ifcvt.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ifcvt.c,v
retrieving revision 1.129
diff -c -3 -p -r1.129 ifcvt.c
*** ifcvt.c 2 Nov 2003 13:56:40 -0000 1.129
--- ifcvt.c 9 Nov 2003 20:30:59 -0000
*************** noce_emit_move_insn (rtx x, rtx y)
*** 700,705 ****
--- 700,715 ----
GET_MODE_BITSIZE (inmode));
}
+ /* Unshare sequence SEQ produced by if conversion. We care to mark
+ all arguments that may be shared with outer instruction stream. */
+ static void
+ unshare_ifcvt_sequence (struct noce_if_info *if_info, rtx seq)
+ {
+ set_used_flags (if_info->x);
+ set_used_flags (if_info->cond);
+ unshare_all_rtl_in_chain (seq);
+ }
+
/* Convert "if (a != b) x = a; else x = b" into "x = a" and
"if (a == b) x = a; else x = b" into "x = b". */
*************** noce_try_move (struct noce_if_info *if_i
*** 734,739 ****
--- 744,750 ----
start_sequence ();
noce_emit_move_insn (if_info->x, y);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
emit_insn_before_setloc (seq, if_info->jump,
INSN_LOCATOR (if_info->insn_a));
*************** noce_try_store_flag (struct noce_if_info
*** 777,782 ****
--- 788,794 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATOR (if_info->insn_a));
*************** noce_try_store_flag_constants (struct no
*** 907,912 ****
--- 919,925 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
if (seq_contains_jump (seq))
*************** noce_try_addcc (struct noce_if_info *if_
*** 956,961 ****
--- 969,975 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
emit_insn_before_setloc (seq, if_info->jump,
INSN_LOCATOR (if_info->insn_a));
*************** noce_try_addcc (struct noce_if_info *if_
*** 994,999 ****
--- 1008,1014 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
if (seq_contains_jump (seq))
*************** noce_try_store_flag_mask (struct noce_if
*** 1046,1051 ****
--- 1061,1067 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
if (seq_contains_jump (seq))
*************** noce_try_cmove (struct noce_if_info *if_
*** 1143,1148 ****
--- 1159,1165 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
emit_insn_before_setloc (seq, if_info->jump,
INSN_LOCATOR (if_info->insn_a));
*************** noce_try_cmove_arith (struct noce_if_inf
*** 1305,1310 ****
--- 1322,1328 ----
noce_emit_move_insn (x, target);
tmp = get_insns ();
+ unshare_ifcvt_sequence (if_info, tmp);
end_sequence ();
emit_insn_before_setloc (tmp, if_info->jump, INSN_LOCATOR (if_info->insn_a));
return TRUE;
*************** noce_try_minmax (struct noce_if_info *if
*** 1550,1555 ****
--- 1568,1574 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
if (seq_contains_jump (seq))
*************** noce_try_abs (struct noce_if_info *if_in
*** 1667,1672 ****
--- 1686,1692 ----
noce_emit_move_insn (if_info->x, target);
seq = get_insns ();
+ unshare_ifcvt_sequence (if_info, seq);
end_sequence ();
if (seq_contains_jump (seq))
*************** noce_process_if_block (struct ce_if_bloc
*** 1988,1995 ****
if (orig_x != x)
{
start_sequence ();
! noce_emit_move_insn (copy_rtx (orig_x), x);
insn_b = get_insns ();
end_sequence ();
emit_insn_after_setloc (insn_b, test_bb->end, INSN_LOCATOR (insn_a));
--- 2008,2017 ----
if (orig_x != x)
{
start_sequence ();
! noce_emit_move_insn (orig_x, x);
insn_b = get_insns ();
+ set_used_flags (orig_x);
+ unshare_all_rtl_in_chain (insn_b);
end_sequence ();
emit_insn_after_setloc (insn_b, test_bb->end, INSN_LOCATOR (insn_a));
Index: rtl.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/rtl.h,v
retrieving revision 1.437
diff -c -3 -p -r1.437 rtl.h
*** rtl.h 18 Oct 2003 18:45:15 -0000 1.437
--- rtl.h 9 Nov 2003 20:30:59 -0000
*************** extern void delete_insns_since (rtx);
*** 2036,2041 ****
--- 2036,2042 ----
extern void mark_reg_pointer (rtx, int);
extern void mark_user_reg (rtx);
extern void reset_used_flags (rtx);
+ extern void set_used_flags (rtx);
extern void reorder_insns (rtx, rtx, rtx);
extern void reorder_insns_nobb (rtx, rtx, rtx);
extern int get_max_uid (void);
*************** extern void set_new_first_and_last_insn
*** 2051,2056 ****
--- 2052,2059 ----
extern void set_new_first_and_last_label_num (int, int);
extern void set_new_last_label_num (int);
extern void unshare_all_rtl_again (rtx);
+ extern void unshare_all_rtl_in_chain (rtx);
+ extern void check_rtl_sharing (void);
extern void set_first_insn (rtx);
extern void set_last_insn (rtx);
extern void link_cc0_insns (rtx);
Index: toplev.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/toplev.c,v
retrieving revision 1.843
diff -c -3 -p -r1.843 toplev.c
*** toplev.c 1 Nov 2003 02:23:44 -0000 1.843
--- toplev.c 9 Nov 2003 20:31:00 -0000
*************** rest_of_compilation (tree decl)
*** 3416,3421 ****
--- 3416,3426 ----
if (optimize > 0 && (flag_regmove || flag_expensive_optimizations))
rest_of_handle_regmove (decl, insns);
+ #ifdef ENABLE_CHECKING
+ /* Last point where memories should be unique. */
+ check_rtl_sharing ();
+ #endif
+
/* Do unconditional splitting before register allocation to allow machine
description to add extra information not needed previously. */
split_all_insns (1);
*************** rest_of_compilation (tree decl)
*** 3613,3618 ****
--- 3618,3628 ----
/* CFG is no longer maintained up-to-date. */
free_bb_for_insn ();
+
+ #ifdef ENABLE_CHECKING
+ check_rtl_sharing ();
+ #endif
+
if (targetm.machine_dependent_reorg != 0)
rest_of_handle_machine_reorg (decl, insns);
More information about the Gcc-patches
mailing list