[committed][PR rtl-optimization/89399] Safely access the source/destination of sets
Jeff Law
law@redhat.com
Thu Apr 4 20:54:00 GMT 2019
As noted in the BZ ree was incorrectly looking at SET_SRC/SET_DEST of
PATTERN of various insns.
In the case where we're looking at something in the candidate list, we
know that all the insns passed the single_set test. So when digging
into something from the candidate list, we should call single_set to
extract the set, then use SET_SRC/SET_DEST to look at the appropriate
parts of the set. That's precisely what this patch does.
Other places within ree.c use get_sub_rtx, but that seems to be for the
insn we want to change rather than items on the candidate list.
get_sub_rtx uses PATTERN (insn) internally, but does so in a reasonably
safe manner.
transform_ifelse also uses PATTERN (insn), but AFAICT only does so for
conditional moves which passed the is_cond_copy_insn test which includes
a single_set test. So those are safe as well.
Bootstrapped and regression tested on a variety of platforms.
Installing on the trunk.
Jeff
-------------- next part --------------
commit 289d708e113e0d579eb78f9b5e63fb6146288cf5
Author: Jeff Law <law@redhat.com>
Date: Thu Apr 4 14:49:53 2019 -0600
PR rtl-optimization/89399
* ree.c (combine_set_extension): Use single_set rather than
digging into PATTERN for items on the candidate list.
(combine_reaching_defs): Likewise.
PR rtl-optimization/89399
* gcc.c-torture/compile/pr89399.c: New test.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 47eeed65ab7..f57c2f9c193 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,10 @@
+2019-04-04 Jeff Law <law@redhat.com>
+
+ PR rtl-optimization/89399
+ * ree.c (combine_set_extension): Use single_set rather than
+ digging into PATTERN for items on the candidate list.
+ (combine_reaching_defs): Likewise.
+
2019-04-04 Richard Sandiford <richard.sandiford@arm.com>
PR rtl-optimization/46590
diff --git a/gcc/ree.c b/gcc/ree.c
index e23e607a5e1..104f8dbf435 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -320,7 +320,7 @@ combine_set_extension (ext_cand *cand, rtx_insn *curr_insn, rtx *orig_set)
rtx orig_src = SET_SRC (*orig_set);
machine_mode orig_mode = GET_MODE (SET_DEST (*orig_set));
rtx new_set;
- rtx cand_pat = PATTERN (cand->insn);
+ rtx cand_pat = single_set (cand->insn);
/* If the extension's source/destination registers are not the same
then we need to change the original load to reference the destination
@@ -778,10 +778,14 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
/* If the destination operand of the extension is a different
register than the source operand, then additional restrictions
are needed. Note we have to handle cases where we have nested
- extensions in the source operand. */
+ extensions in the source operand.
+
+ Candidate insns are known to be single_sets, via the test in
+ find_removable_extensions. So we continue to use single_set here
+ rather than get_sub_rtx. */
+ rtx set = single_set (cand->insn);
bool copy_needed
- = (REGNO (SET_DEST (PATTERN (cand->insn)))
- != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))));
+ = (REGNO (SET_DEST (set)) != REGNO (get_extended_src_reg (SET_SRC (set))));
if (copy_needed)
{
/* Considering transformation of
@@ -816,8 +820,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
return false;
- machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn)));
- rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)));
+ machine_mode dst_mode = GET_MODE (SET_DEST (set));
+ rtx src_reg = get_extended_src_reg (SET_SRC (set));
/* Ensure we can use the src_reg in dst_mode (needed for
the (set (reg1) (reg2)) insn mentioned above). */
@@ -852,9 +856,9 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
|| !REG_P (SET_DEST (*dest_sub_rtx)))
return false;
- rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (PATTERN (cand->insn))),
+ rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (set)),
REGNO (SET_DEST (*dest_sub_rtx)));
- if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn))))
+ if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (set)))
return false;
/* On RISC machines we must make sure that changing the mode of SRC_REG
@@ -877,10 +881,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
/* The destination register of the extension insn must not be
used or set between the def_insn and cand->insn exclusive. */
- if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn, cand->insn)
- || reg_set_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn, cand->insn))
+ if (reg_used_between_p (SET_DEST (set), def_insn, cand->insn)
+ || reg_set_between_p (SET_DEST (set), def_insn, cand->insn))
return false;
/* We must be able to copy between the two registers. Generate,
@@ -894,11 +896,10 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
is different than in the code to emit the copy as we have not
modified the defining insn yet. */
start_sequence ();
- rtx pat = PATTERN (cand->insn);
- rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
- REGNO (get_extended_src_reg (SET_SRC (pat))));
- rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
- REGNO (SET_DEST (pat)));
+ rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (set)),
+ REGNO (get_extended_src_reg (SET_SRC (set))));
+ rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (set)),
+ REGNO (SET_DEST (set)));
emit_move_insn (new_dst, new_src);
rtx_insn *insn = get_insns ();
@@ -912,7 +913,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
return false;
while (REG_P (SET_SRC (*dest_sub_rtx))
- && (REGNO (SET_SRC (*dest_sub_rtx)) == REGNO (SET_DEST (pat))))
+ && (REGNO (SET_SRC (*dest_sub_rtx)) == REGNO (SET_DEST (set))))
{
/* Considering transformation of
(set (reg2) (expression))
@@ -955,7 +956,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
implicitly sets it in word mode. */
if (WORD_REGISTER_OPERATIONS && known_lt (prec, BITS_PER_WORD))
{
- struct df_link *uses = get_uses (def_insn2, SET_DEST (pat));
+ struct df_link *uses = get_uses (def_insn2, SET_DEST (set));
if (!uses)
break;
@@ -973,10 +974,8 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
used or set between the def_insn2 and def_insn exclusive.
Likewise for the other reg, i.e. check both reg1 and reg2
in the above comment. */
- if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn2, def_insn)
- || reg_set_between_p (SET_DEST (PATTERN (cand->insn)),
- def_insn2, def_insn)
+ if (reg_used_between_p (SET_DEST (set), def_insn2, def_insn)
+ || reg_set_between_p (SET_DEST (set), def_insn2, def_insn)
|| reg_used_between_p (src_reg, def_insn2, def_insn)
|| reg_set_between_p (src_reg, def_insn2, def_insn))
break;
@@ -991,13 +990,12 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
{
machine_mode mode;
- rtx set;
if (state->modified[INSN_UID (cand->insn)].kind
!= (cand->code == ZERO_EXTEND
? EXT_MODIFIED_ZEXT : EXT_MODIFIED_SEXT)
|| state->modified[INSN_UID (cand->insn)].mode != cand->mode
- || (set = single_set (cand->insn)) == NULL_RTX)
+ || (set == NULL_RTX))
return false;
mode = GET_MODE (SET_DEST (set));
gcc_assert (GET_MODE_UNIT_SIZE (mode)
@@ -1320,9 +1318,9 @@ find_and_remove_re (void)
we do not allow the optimization of extensions where
the source and destination registers do not match. Thus
checking REG_P here is correct. */
- if (REG_P (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0))
- && (REGNO (SET_DEST (PATTERN (curr_cand->insn)))
- != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0))))
+ rtx set = single_set (curr_cand->insn);
+ if (REG_P (XEXP (SET_SRC (set), 0))
+ && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
{
reinsn_copy_list.safe_push (curr_cand->insn);
reinsn_copy_list.safe_push (state.defs_list[0]);
@@ -1356,13 +1354,13 @@ find_and_remove_re (void)
defining insn was used to eliminate a second extension
that was wider than the first. */
rtx sub_rtx = *get_sub_rtx (def_insn);
- rtx pat = PATTERN (curr_insn);
+ rtx set = single_set (curr_insn);
rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
- REGNO (XEXP (SET_SRC (pat), 0)));
+ REGNO (XEXP (SET_SRC (set), 0)));
rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
- REGNO (SET_DEST (pat)));
- rtx set = gen_rtx_SET (new_dst, new_src);
- emit_insn_after (set, def_insn);
+ REGNO (SET_DEST (set)));
+ rtx new_set = gen_rtx_SET (new_dst, new_src);
+ emit_insn_after (new_set, def_insn);
}
/* Delete all useless extensions here in one sweep. */
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 7afa590a8ef..910d55b56f5 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-04 Jeff Law <law@redhat.com>
+
+ PR rtl-optimization/89399
+ * gcc.c-torture/compile/pr89399.c: New test.
+
2019-04-04 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/65619
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr89399.c b/gcc/testsuite/gcc.c-torture/compile/pr89399.c
new file mode 100644
index 00000000000..ce8fbfc4fb8
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr89399.c
@@ -0,0 +1,10 @@
+extern int bar(void);
+
+short s;
+
+int foo(void)
+{
+ s = bar();
+ return s;
+}
+
More information about the Gcc-patches
mailing list