Hi,
this is a wrong code regression at -O2 on the mainline for Alpha coming from
the REE pass (Alpha is one of the 3 architectures enabling REE at -O2 but I'm
probably going to enable it for 64-bit SPARC too). The problem arises when a
copy is needed in combine_reaching_defs:
/* 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. */
bool copy_needed
= (REGNO (SET_DEST (PATTERN (cand->insn)))
!= REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))));
if (copy_needed)
{
/* Considering transformation of
(set (reg1) (expression))
...
(set (reg2) (any_extend (reg1)))
into
(set (reg2) (any_extend (expression)))
(set (reg1) (reg2))
Note that the mode of reg1 is changed by the transformation, e.g. from QImode
to DImode, so the transformation can change the upper bits of reg1. But there
is no general check that this will not affect the other reaching uses of reg1:
(set (reg1:QI) (expression:QI))
...
(set (reg2:DI) (any_extend:DI (reg1:QI)))
...
(use (reg1:DI))
I initially thought that this would only matter for WORD_REGISTER_OPERATIONS
targets like Alpha, where the first set in QImode can implicitly set the whole
DImode register so the use reads well-defined upper bits, but I now wonder
whether this also matters for !WORD_REGISTER_OPERATIONS targets, e.g. x86:
(set (reg1:DI) ...
...
(set (reg1:QI) (expression:QI))
...
(set (reg2:DI) (any_extend:DI (reg1:QI)))
...
(use (reg1:DI))
where the use reads well-defined upper bits from the very first set.
Hence the 2 attached versions of the fix: pr78437-1.diff only adds the check
for WORD_REGISTER_OPERATIONS targets and was confirmed by Uros as fixing the
regression of gcc.dg/atomic/stdatomic-compare-exchange-[1,2].c on Alpha, while
pr78437-2.diff adds the check for all targets and was also tested on x86-64.
Thoughts?
PR rtl-optimization/78437
* ree.c (get_uses): New function.
(combine_reaching_defs): When a copy is needed, return false if any
reaching use of the source register reads it in a mode larger than
the mode it is set in[ and if WORD_REGISTER_OPERATIONS is true].
--
Eric BotcazouIndex: ree.c
===================================================================
--- ree.c (revision 242632)
+++ ree.c (working copy)
@@ -499,6 +499,35 @@ get_defs (rtx_insn *insn, rtx reg, vec<r
return ref_chain;
}
+/* Get all the reaching uses of an instruction. The uses are desired for REG
+ set in INSN. Return use list or NULL if a use is missing or irregular. */
+
+static struct df_link *
+get_uses (rtx_insn *insn, rtx reg)
+{
+ df_ref def;
+ struct df_link *ref_chain, *ref_link;
+
+ FOR_EACH_INSN_DEF (def, insn)
+ if (REGNO (DF_REF_REG (def)) == REGNO (reg))
+ break;
+
+ gcc_assert (def != NULL);
+
+ ref_chain = DF_REF_CHAIN (def);
+
+ for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ {
+ /* Problem getting some use for this instruction. */
+ if (ref_link->ref == NULL)
+ return NULL;
+ if (DF_REF_CLASS (ref_link->ref) != DF_REF_REGULAR)
+ return NULL;
+ }
+
+ return ref_chain;
+}
+
/* Return true if INSN is
(SET (reg REGNO (def_reg)) (if_then_else (cond) (REG x1) (REG x2)))
and store x1 and x2 in REG_1 and REG_2. */
@@ -827,6 +856,23 @@ combine_reaching_defs (ext_cand *cand, c
if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn))))
return false;
+ /* On RISC machines we must make sure that changing the mode of SRC_REG
+ as destination register will not affect its reaching uses, which may
+ read its value in a larger mode because DEF_INSN implicitly sets it
+ in word mode. */
+ const unsigned int prec
+ = GET_MODE_PRECISION (GET_MODE (SET_DEST (*dest_sub_rtx)));
+ if (WORD_REGISTER_OPERATIONS && prec < BITS_PER_WORD)
+ {
+ struct df_link *uses = get_uses (def_insn, src_reg);
+ if (!uses)
+ return false;
+
+ for (df_link *use = uses; use; use = use->next)
+ if (GET_MODE_PRECISION (GET_MODE (*DF_REF_LOC (use->ref))) > prec)
+ return false;
+ }
+
/* 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)),
Index: ree.c
===================================================================
--- ree.c (revision 242632)
+++ ree.c (working copy)
@@ -499,6 +499,35 @@ get_defs (rtx_insn *insn, rtx reg, vec<r
return ref_chain;
}
+/* Get all the reaching uses of an instruction. The uses are desired for REG
+ set in INSN. Return use list or NULL if a use is missing or irregular. */
+
+static struct df_link *
+get_uses (rtx_insn *insn, rtx reg)
+{
+ df_ref def;
+ struct df_link *ref_chain, *ref_link;
+
+ FOR_EACH_INSN_DEF (def, insn)
+ if (REGNO (DF_REF_REG (def)) == REGNO (reg))
+ break;
+
+ gcc_assert (def != NULL);
+
+ ref_chain = DF_REF_CHAIN (def);
+
+ for (ref_link = ref_chain; ref_link; ref_link = ref_link->next)
+ {
+ /* Problem getting some use for this instruction. */
+ if (ref_link->ref == NULL)
+ return NULL;
+ if (DF_REF_CLASS (ref_link->ref) != DF_REF_REGULAR)
+ return NULL;
+ }
+
+ return ref_chain;
+}
+
/* Return true if INSN is
(SET (reg REGNO (def_reg)) (if_then_else (cond) (REG x1) (REG x2)))
and store x1 and x2 in REG_1 and REG_2. */
@@ -827,6 +856,24 @@ combine_reaching_defs (ext_cand *cand, c
if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn))))
return false;
+ /* We must make sure that changing the mode of SRC_REG as destination
+ register will not affect its reaching uses, which may read its value
+ in a larger mode because either 1) DEF_INSN implicitly sets it in
+ word mode for WORD_REGISTER_OPERATIONS targets or 2) DEF_INSN does
+ not modify its upper bits for !WORD_REGISTER_OPERATIONS targets. */
+ const unsigned int prec
+ = GET_MODE_PRECISION (GET_MODE (SET_DEST (*dest_sub_rtx)));
+ if (prec < BITS_PER_WORD)
+ {
+ struct df_link *uses = get_uses (def_insn, src_reg);
+ if (!uses)
+ return false;
+
+ for (df_link *use = uses; use; use = use->next)
+ if (GET_MODE_PRECISION (GET_MODE (*DF_REF_LOC (use->ref))) > prec)
+ return false;
+ }
+
/* 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)),
