Hi,
The patch is updated. If there is no df_live, we still can not
guarantee the correctness. So the new patch just checks the
DF_INSN_DEFS.
Bootstrap and no make check regression on x86-64.
Bootstrap on ARM chrome book.
Is it OK?
Thanks!
-Zhenqiang
Changelog:
2013-07-05 Zhenqiang Chen <zhenqiang.c...@linaro.org>
PR target/57637
* function.c (move_insn_for_shrink_wrap): Check DF_INSN_DEFS.
diff --git a/gcc/function.c b/gcc/function.c
index 3e33fc7..28f1b78 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5524,12 +5524,40 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
SET_REGNO_REG_SET (live_in, i);
}
+ /* DF_LR_BB_INFO (bb)->def does not cover the DF_REF_PARTIAL and
+ DF_REF_CONDITIONAL def. So recheck the DF_INSN_DEFS. */
+ if (next_block)
+ {
+ rtx x;
+ df_ref *def_rec;
+
+ FOR_BB_INSNS(bb, x)
+ {
+ if (!NONDEBUG_INSN_P (x))
+ continue;
+
+ for (def_rec = DF_INSN_DEFS (x); *def_rec; def_rec++)
+ {
+ df_ref def = *def_rec;
+ unsigned int regno = DF_REF_REGNO (def);
+
+ for (i = dregno; i < end_dregno; i++)
+ if (i == regno)
+ goto DONE;
+ for (i = sregno; i < end_sregno; i++)
+ if (i == regno)
+ goto DONE;
+ }
+ }
+ }
+
/* If we don't need to add the move to BB, look for a single
successor block. */
if (next_block)
next_block = next_block_for_reg (next_block, dregno, end_dregno);
}
while (next_block);
+DONE:
/* BB now defines DEST. It only uses the parts of DEST that overlap SRC
(next loop). */
On 28 June 2013 15:02, Zhenqiang Chen <zhenqiang.c...@linaro.org> wrote:
> On 27 June 2013 21:10, Richard Earnshaw <rearn...@arm.com> wrote:
>> On 27/06/13 10:02, Zhenqiang Chen wrote:
>>>
>>> Hi,
>>>
>>> Shrink-wrap optimization sinks some instructions for more
>>> opportunities. It uses DF_LR_BB_INFO (bb)->def to check whether BB
>>> clobbers SRC. But for ARM, gcc might generate cond_exec insns before
>>> shrink-wrapping. And DF_LR_BB_INFO (bb)->def does not include def info
>>> from cond_exec insns. So the check in function
>>> move_insn_for_shrink_wrap is not enough.
>>>
>>> The patch is to add more check bases on DF_LIVE_BB_INFO (bb)->gen if
>>> df-live is available.
>>>
>>> Bootstrap and no make check regression on x86-64 and Panda board.
>>>
>>> Is is OK for trunk?
>>>
>>> Thanks!
>>> -Zhenqiang
>>>
>>> ChangeLog:
>>> 2013-06-27 Zhenqiang Chen <zhenqiang.c...@linaro.org>
>>>
>>> PR target/57637
>>> * function.c (move_insn_for_shrink_wrap): Check
>>> DF_LIVE_BB_INFO (bb)->gen.
>>
>>
>> First off, this isn't really my area, so all this might be off base. Did you
>> really mean Richard Sandiford?
>>
>> The code you're looking at here looks a bit odd. We have
>>
>> live_out = df_get_live_out (bb);
>> live_in = df_get_live_in (next_block);
>> bb = next_block;
>>
>> /* Check whether BB uses DEST or clobbers DEST. We need to add
>> INSN to BB if so. Either way, DEST is no longer live on entry,
>> except for any part that overlaps SRC (next loop). */
>> bb_uses = &DF_LR_BB_INFO (bb)->use;
>>
>> bb_defs = &DF_LR_BB_INFO (bb)->def;
>>
>> The setting of live_out and live in uses
>>
>> if (df_live)
>> return DF_LIVE_OUT (bb);
>> else
>> return DF_LR_OUT (bb);
>>
>> but for bb_uses and bb_defs, we unconditionally use the LR problem and never
>> consider live.
>>
>> That seems strange to me.
>>
>> Perhaps a better fix would be to set bb_uses and bb_defs based on whether or
>> not df_live was valid. ie
>>
>> live_out = df_get_live_out (bb);
>> live_in = df_get_live_in (next_block);
>> bb = next_block;
>>
>> /* Check whether BB uses DEST or clobbers DEST. We need to add
>> INSN to BB if so. Either way, DEST is no longer live on entry,
>> except for any part that overlaps SRC (next loop). */
>> if (df_live)
>> {
>> bb_uses = &DF_LIVE_BB_INFO (bb)->use;
>> bb_defs = &DF_LIVE_BB_INFO (bb)->def;
>> }
>> else
>> {
>> bb_uses = &DF_LR_BB_INFO (bb)->use;
>>
>> bb_defs = &DF_LR_BB_INFO (bb)->def;
>> }
>
> Thanks for the comments.
>
> DF_LIVE_BB_INFO includes "gen" and "kill", not "def" and "use". "gen"
> from df_live does not cover all the information of "def" from df_lr. I
> had tried to set bb_defs to &DF_LIVE_BB_INFO (bb)->gen. But x86-64
> bootstrap failed.
>
> -Zhenqiang
>
>>>
>>> diff --git a/gcc/function.c b/gcc/function.c
>>> index 3e33fc7..08ca4a1 100644
>>> --- a/gcc/function.c
>>> +++ b/gcc/function.c
>>> @@ -5508,7 +5508,8 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
>>> bb_defs = &DF_LR_BB_INFO (bb)->def;
>>> for (i = dregno; i < end_dregno; i++)
>>> {
>>> - if (REGNO_REG_SET_P (bb_uses, i) || REGNO_REG_SET_P (bb_defs,
>>> i))
>>> + if (REGNO_REG_SET_P (bb_uses, i) || REGNO_REG_SET_P (bb_defs, i)
>>> + || (df_live && REGNO_REG_SET_P (&DF_LIVE_BB_INFO (bb)->gen,
>>> i)))
>>> next_block = NULL;
>>> CLEAR_REGNO_REG_SET (live_out, i);
>>> CLEAR_REGNO_REG_SET (live_in, i);
>>> @@ -5518,7 +5519,8 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
>>> Either way, SRC is now live on entry. */
>>> for (i = sregno; i < end_sregno; i++)
>>> {
>>> - if (REGNO_REG_SET_P (bb_defs, i))
>>> + if (REGNO_REG_SET_P (bb_defs, i)
>>> + || (df_live && REGNO_REG_SET_P (&DF_LIVE_BB_INFO (bb)->gen,
>>> i)))
>>> next_block = NULL;
>>> SET_REGNO_REG_SET (live_out, i);
>>> SET_REGNO_REG_SET (live_in, i);
>>>
>>
>>
