On 11/10/14 13:44, Felix Yang wrote:
Hello Jeff,
I see that you have improved the RTL typesafety issue for ira.c,
so I rebased this patch
on the latest trunk and change to use the new list walking interface.
Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
OK for trunk?
Hi Felix,
I believe your patch causes a regression for arm-none-eabi.
FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
FAIL: gcc.target/arm/pr43920-2.c scan-assembler-times pop 2
This happens because your patch stops reuse of code for
" return -1;" statements in pr43920-2.c.
As far as I investigated, your patch prevents adding "(expr_list (-1)
(nil)" in ira pass, which prevents jump2 optimization from happening.
So before, in ira pass I could see:
"(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
(const_int -1 [0xffffffffffffffff]))
/work/fsf-trunk-ref-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20 613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0xffffffffffffffff])
(nil)))"
But with your patch I get
"(insn 9 53 34 8 (set (reg:SI 110 [ D.5322 ])
(const_int -1 [0xffffffffffffffff]))
/work/fsf-trunk-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
615 {*thumb2_movsi_vfp}
(nil))"
This causes a code generation regression and needs to be fixed.
Kind regards,
Alex
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog (revision 216116)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,14 @@
+2014-10-11 Felix Yang <felix.y...@huawei.com>
+ Jeff Law <l...@redhat.com>
+
+ * ira.c (struct equivalence): Change member "is_arg_equivalence"
and "replace"
+ into boolean bitfields; turn member "loop_depth" into a short
integer; add new
+ member "no_equiv" and "reserved".
+ (no_equiv): Set no_equiv of struct equivalence if register is marked
+ as having no known equivalence.
+ (update_equiv_regs): Check all definitions for a multiple-set
+ register to make sure that the RHS have the same value.
+
2014-10-11 Martin Liska <mli...@suse.cz>
PR/63376
Index: gcc/ira.c
===================================================================
--- gcc/ira.c (revision 216116)
+++ gcc/ira.c (working copy)
@@ -2902,12 +2902,14 @@ struct equivalence
/* Loop depth is used to recognize equivalences which appear
to be present within the same loop (or in an inner loop). */
- int loop_depth;
+ short loop_depth;
/* Nonzero if this had a preexisting REG_EQUIV note. */
- int is_arg_equivalence;
+ unsigned char is_arg_equivalence : 1;
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
- char replace;
+ unsigned char replace : 1;
+ /* Set if this register has no known equivalence. */
+ unsigned char no_equiv : 1;
};
/* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
@@ -3255,6 +3257,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
if (!REG_P (reg))
return;
regno = REGNO (reg);
+ reg_equiv[regno].no_equiv = 1;
list = reg_equiv[regno].init_insns;
if (list && list->insn () == NULL)
return;
@@ -3381,7 +3384,7 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == 0)
+ if (set == NULL_RTX)
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
@@ -3476,16 +3479,49 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+ {
+ bool equal_p = true;
+ rtx_insn_list *list;
+
+ /* If we have already processed this pseudo and determined it
+ can not have an equivalence, then honor that decision. */
+ if (reg_equiv[regno].no_equiv)
+ continue;
+
+ if (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
- reg_equiv[regno].replacement))))
- {
- no_equiv (dest, set, NULL);
- continue;
+ reg_equiv[regno].replacement)))
+ {
+ no_equiv (dest, set, NULL);
+ continue;
+ }
+
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = list->next ())
+ {
+ rtx note_tmp;
+ rtx_insn *insn_tmp;
+
+ insn_tmp = list->insn ();
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+ gcc_assert (note_tmp);
+ if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+ {
+ equal_p = false;
+ break;
+ }
+ }
+
+ if (! equal_p)
+ {
+ no_equiv (dest, set, NULL);
+ continue;
+ }
}
+
/* Record this insn as initializing this register. */
reg_equiv[regno].init_insns
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
@@ -3514,10 +3550,9 @@ update_equiv_regs (void)
a register used only in one basic block from a MEM. If so, and the
MEM remains unchanged for the life of the register, add a REG_EQUIV
note. */
-
note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
- if (note == 0 && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
+ if (note == NULL_RTX && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
&& MEM_P (SET_SRC (set))
&& validate_equiv_mem (insn, dest, SET_SRC (set)))
note = set_unique_reg_note (insn, REG_EQUIV, copy_rtx (SET_SRC
(set)));
@@ -3547,7 +3582,7 @@ update_equiv_regs (void)
reg_equiv[regno].replacement = x;
reg_equiv[regno].src_p = &SET_SRC (set);
- reg_equiv[regno].loop_depth = loop_depth;
+ reg_equiv[regno].loop_depth = (short) loop_depth;
/* Don't mess with things live during setjmp. */
if (REG_LIVE_LENGTH (regno) >= 0 && optimize)
@@ -3677,7 +3712,7 @@ update_equiv_regs (void)
rtx equiv_insn;
if (! reg_equiv[regno].replace
- || reg_equiv[regno].loop_depth < loop_depth
+ || reg_equiv[regno].loop_depth < (short) loop_depth
/* There is no sense to move insns if live range
shrinkage or register pressure-sensitive
scheduling were done because it will not
Cheers,
Felix
On Tue, Sep 30, 2014 at 5:44 AM, Jeff Law <l...@redhat.com> wrote:
On 09/27/14 08:48, Felix Yang wrote:
Thanks for the explaination.
I have changed the loop_depth into a short interger hoping that we can
save some memory :-)
Thanks.
Attached please find the updated patch. Bootstrapped and reg-tested on
x86_64-suse-linux.
Please do a final revew once the assignment is ready.
As for the new list walking interface, I choose the function
"no_equiv" and tried the "checked cast" way.
The bad news is that GCC failed to bootstrap with the following change:
Index: ira.c
===================================================================
--- ira.c (revision 215536)
+++ ira.c (working copy)
@@ -3242,12 +3242,12 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
void *data ATTRIBUTE_UNUSED)
{
int regno;
- rtx list;
+ rtx_insn_list *list;
if (!REG_P (reg))
return;
regno = REGNO (reg);
- list = reg_equiv[regno].init_insns;
+ list = as_a <rtx_insn_list *> (reg_equiv[regno].init_insns);
if (list == const0_rtx)
return;
reg_equiv[regno].init_insns = const0_rtx;
@@ -3258,9 +3258,9 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
return;
ira_reg_equiv[regno].defined_p = false;
ira_reg_equiv[regno].init_insns = NULL;
- for (; list; list = XEXP (list, 1))
+ for (; list; list = list->next ())
{
- rtx insn = XEXP (list, 0);
+ rtx_insn *insn = list->insn ();
remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
}
}
Yea. I'm going to post a patch shortly to go ahead with this conversion.
There's a couple issues that come into play.
First const0_rtx is not an INSN, so we *really* don't want it in the INSN
field of an INSN_LIST. That's probably the ICE you're seeing.
const0_rtx is being used to mark pseudos which we've already determined
can't have a valid equivalence. So we just need a different marker. That
different marker must be embeddable in an INSN_LIST node. The easiest is
just a NULL insn ;-)
The other tests for the const0_rtx marker in ira.c need relatively trivial
updating. And in the end we don't need the checked cast at all ;-)
Jeff
