This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: patch to solve PR37535
H.J. Lu wrote:
On Mon, Sep 22, 2008 at 12:20 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
Richard Sandiford wrote:
Vladimir Makarov <vmakarov@redhat.com> writes:
Richard Sandiford wrote:
Vladimir Makarov <vmakarov@redhat.com> writes:
The following patch solves the PR37535. The analysis of the problem
can be found on
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37535
Thanks for the patch. But my understanding is that (clobber (reg X))
is an earlyclobber for register-allocation purposes, not an output
clobber. C.f. the following bit of rtl.texi:
When a @code{clobber} expression for a register appears inside a
@code{parallel} with other side effects, the register allocator
guarantees that the register is unoccupied both before and after that
insn.
It does go on to say:
... However, the reload phase may allocate a register used for one of
the inputs unless the @samp{&} constraint is specified for the
selected
alternative (@pxref{Modifiers}). You can clobber either a specific
hard
register, a pseudo register, or a @code{scratch} expression; in the
latter two cases, GCC will allocate a hard register that is available
there for use as a temporary.
It is hard to say me why we should provide what is written in the first
phrase, when it is not guaranteed in reload. It really has no sense to me.
Probably I miss something here.
I suppose it all boils down to: is an _unmatched_ (clobber (reg X))
an output clobber or an early clobber? Because it's unmatched, there
are no constraints to tell us which.
Ok. I think my patch is better then. Because if there is no one early
clobber insn alternative, the patch permits to use clobber hard reg for
input operands in RA (the code would better although to be honest I did not
see the difference out of noise when I tested my patch on SPEC2000 on x86
but the code size is insignificantly smaller). So instead of your patch, we
could
1. use my patch and
2. modify the documentation to something like that
Hi Vladimir,
Your patch:
http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01176.html
caused this regression
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37598
I don't think it can be applied ASIS. But Richard's
patch fixes PR 37535 and doesn't have any regressions
on Linux/ia32, Linux/ia64 and Linux/Intel64.
I missed that clobbers in asm are actually *early* clobbers. The new
version of the patch removes the regression. It is mainly an additional
code in mark_early_clobbers to deal with clobbers in asm insns.
The changelog should be the same.
I'll test the new version of the patch more and submit it for the approval.
Index: ira-lives.c
===================================================================
--- ira-lives.c (revision 140564)
+++ ira-lives.c (working copy)
@@ -209,20 +209,15 @@ clear_allocno_live (ira_allocno_t a)
sparseset_clear_bit (allocnos_live, ALLOCNO_NUM (a));
}
-/* Mark the register referenced by use or def REF as live
- Store a 1 in hard_regs_live or allocnos_live for this register or
- the corresponding allocno, record how many consecutive hardware
- registers it actually needs. */
-
+/* Mark the register REG as live. Store a 1 in hard_regs_live or
+ allocnos_live for this register or the corresponding allocno,
+ record how many consecutive hardware registers it actually
+ needs. */
static void
-mark_ref_live (struct df_ref *ref)
+mark_reg_live (rtx reg)
{
- rtx reg;
int regno;
- reg = DF_REF_REG (ref);
- if (GET_CODE (reg) == SUBREG)
- reg = SUBREG_REG (reg);
gcc_assert (REG_P (reg));
regno = REGNO (reg);
@@ -269,32 +264,25 @@ mark_ref_live (struct df_ref *ref)
}
}
-/* Return true if the definition described by DEF conflicts with the
- instruction's inputs. */
-static bool
-def_conflicts_with_inputs_p (struct df_ref *def)
+/* Mark the register referenced by use or def REF as live. */
+static void
+mark_ref_live (struct df_ref *ref)
{
- /* Conservatively assume that the condition is true for all clobbers. */
- return DF_REF_FLAGS_IS_SET (def, DF_REF_MUST_CLOBBER);
+ rtx reg;
+
+ reg = DF_REF_REG (ref);
+ if (GET_CODE (reg) == SUBREG)
+ reg = SUBREG_REG (reg);
+ mark_reg_live (reg);
}
-/* Mark the register referenced by definition DEF as dead, if the
- definition is a total one. Store a 0 in hard_regs_live or
+/* Mark the register REG as dead. Store a 0 in hard_regs_live or
allocnos_live for the register. */
static void
-mark_ref_dead (struct df_ref *def)
+mark_reg_dead (rtx reg)
{
- unsigned int i;
- rtx reg;
int regno;
- if (DF_REF_FLAGS_IS_SET (def, DF_REF_PARTIAL)
- || DF_REF_FLAGS_IS_SET (def, DF_REF_CONDITIONAL))
- return;
-
- reg = DF_REF_REG (def);
- if (GET_CODE (reg) == SUBREG)
- reg = SUBREG_REG (reg);
gcc_assert (REG_P (reg));
regno = REGNO (reg);
@@ -312,6 +300,7 @@ mark_ref_dead (struct df_ref *def)
}
else if (! TEST_HARD_REG_BIT (ira_no_alloc_regs, regno))
{
+ unsigned int i;
int last = regno + hard_regno_nregs[regno][GET_MODE (reg)];
enum reg_class cover_class;
@@ -343,6 +332,71 @@ mark_ref_dead (struct df_ref *def)
}
}
+/* Mark the register referenced by definition DEF as dead, if the
+ definition is a total one. */
+static void
+mark_ref_dead (struct df_ref *def)
+{
+ rtx reg;
+
+ if (DF_REF_FLAGS_IS_SET (def, DF_REF_PARTIAL)
+ || DF_REF_FLAGS_IS_SET (def, DF_REF_CONDITIONAL))
+ return;
+
+ reg = DF_REF_REG (def);
+ if (GET_CODE (reg) == SUBREG)
+ reg = SUBREG_REG (reg);
+ mark_reg_dead (reg);
+}
+
+/* Mark early clobber registers of the current INSN as live (if
+ LIVE_P) or dead. Return true if there are such registers. */
+static bool
+mark_early_clobbers (rtx insn, bool live_p)
+{
+ int alt;
+ int def;
+ struct df_ref **def_rec;
+ bool set_p = false;
+ bool asm_p = asm_noperands (PATTERN (insn)) >= 0;
+
+ if (asm_p)
+ for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
+ if (DF_REF_FLAGS_IS_SET (*def_rec, DF_REF_MUST_CLOBBER))
+ {
+ if (live_p)
+ mark_ref_live (*def_rec);
+ else
+ mark_ref_dead (*def_rec);
+ set_p = true;
+ }
+
+ for (def = 0; def < recog_data.n_operands; def++)
+ {
+ rtx dreg = recog_data.operand[def];
+
+ if (GET_CODE (dreg) == SUBREG)
+ dreg = SUBREG_REG (dreg);
+ if (! REG_P (dreg))
+ continue;
+
+ for (alt = 0; alt < recog_data.n_alternatives; alt++)
+ if ((recog_op_alt[def][alt].earlyclobber)
+ && (recog_op_alt[def][alt].cl != NO_REGS))
+ break;
+
+ if (alt >= recog_data.n_alternatives)
+ continue;
+
+ if (live_p)
+ mark_reg_live (dreg);
+ else
+ mark_reg_dead (dreg);
+ set_p = true;
+ }
+ return set_p;
+}
+
/* Checks that CONSTRAINTS permits to use only one hard register. If
it is so, the function returns the class of the hard register.
Otherwise it returns NO_REGS. */
@@ -580,6 +634,7 @@ process_bb_node_lives (ira_loop_tree_nod
bitmap_iterator bi;
bitmap reg_live_out;
unsigned int px;
+ bool set_p;
bb = loop_tree_node->bb;
if (bb != NULL)
@@ -698,6 +753,7 @@ process_bb_node_lives (ira_loop_tree_nod
}
extract_insn (insn);
+ preprocess_constraints ();
process_single_reg_class_operands (false, freq);
/* See which defined values die here. */
@@ -733,19 +789,12 @@ process_bb_node_lives (ira_loop_tree_nod
for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
mark_ref_live (*use_rec);
- /* If any defined values conflict with the inputs, mark those
- defined values as live. */
- for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
- if (def_conflicts_with_inputs_p (*def_rec))
- mark_ref_live (*def_rec);
+ set_p = mark_early_clobbers (insn, true);
process_single_reg_class_operands (true, freq);
- /* See which of the defined values we marked as live are dead
- before the instruction. */
- for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
- if (def_conflicts_with_inputs_p (*def_rec))
- mark_ref_dead (*def_rec);
+ if (set_p)
+ mark_early_clobbers (insn, false);
curr_point++;
}