Hi Jakub

I have run it on 4.6, it passes the following testing:

x86-64 bootstrap
x86-64 regression test
regression test on arm qemu

Is it OK for gcc4.6?

Ahmad, is it OK for google/gcc-4_6/ and google/gcc-4_6-mobile ?

thanks
Carrot

On Wed, Sep 12, 2012 at 2:01 PM, Carrot Wei <car...@google.com> wrote:
> Hi Jakub
>
> The same problem also affects gcc4.6,
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54398. Could this be
> ported to 4.6 branch?
>
> thanks
> Carrot
>
> On Mon, Feb 13, 2012 at 11:54 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>>
>> On Wed, Jan 04, 2012 at 05:21:38PM +0000, Marcus Shawcroft wrote:
>> > Alias analysis by DSE based on CSELIB expansion assumes that
>> > references to the stack frame from different base registers (ie FP, SP)
>> > never alias.
>> >
>> > The comment block in cselib explains that cselib does not allow
>> > substitution of FP, SP or SFP specifically in order not to break DSE.
>>
>> Looks reasonable, appart from coding style (no spaces around -> and
>> no {} around return p->loc;), I just wonder if having a separate
>> loop in expand_loc just for this isn't too expensive.  On sane targets
>> IMHO hard frame pointer in the prologue should be initialized from sp, not
>> the other way around, thus hard frame pointer based VALUEs should have
>> hard frame pointer earlier in the locs list (when there is
>> hfp = sp (+ optionally some const)
>> insn, we first cselib_lookup_from_insn the rhs and add to locs
>> of the new VALUE (plus (VALUE of sp) (const_int)), then process the
>> lhs and add it to locs, moving the plus to locs->next).
>> So I think the following patch could be enough (bootstrapped/regtested
>> on x86_64-linux and i686-linux).
>> There is AVR though, which has really weirdo prologue - PR50063,
>> but I think it should just use UNSPEC for that or something similar,
>> setting sp from hfp seems unnecessary and especially for values with long
>> locs chains could make cselib more expensive.
>>
>> Richard, what do you think about this?
>>
>> 2012-02-13  Jakub Jelinek  <ja...@redhat.com>
>>
>>         * cselib.c (expand_loc): Return sp, fp, hfp or cfa base reg right
>>         away if seen.
>>
>> --- gcc/cselib.c.jj     2012-02-13 11:07:15.000000000 +0100
>> +++ gcc/cselib.c        2012-02-13 18:15:17.531776145 +0100
>> @@ -1372,8 +1372,18 @@ expand_loc (struct elt_loc_list *p, stru
>>    unsigned int regno = UINT_MAX;
>>    struct elt_loc_list *p_in = p;
>>
>> -  for (; p; p = p -> next)
>> +  for (; p; p = p->next)
>>      {
>> +      /* Return these right away to avoid returning stack pointer based
>> +        expressions for frame pointer and vice versa, which is something
>> +        that would confuse DSE.  See the comment in
>> cselib_expand_value_rtx_1
>> +        for more details.  */
>> +      if (REG_P (p->loc)
>> +         && (REGNO (p->loc) == STACK_POINTER_REGNUM
>> +             || REGNO (p->loc) == FRAME_POINTER_REGNUM
>> +             || REGNO (p->loc) == HARD_FRAME_POINTER_REGNUM
>> +             || REGNO (p->loc) == cfa_base_preserved_regno))
>> +       return p->loc;
>>        /* Avoid infinite recursion trying to expand a reg into a
>>          the same reg.  */
>>        if ((REG_P (p->loc))
>>
>>
>>         Jakub

Reply via email to