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);
>>
>
>
