| Message ID | CAOvf_xwYSxEKbXssBAH2KBUZQzMDJzBHXvM5R4tJZ=hvihccKw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 13, 2014 at 5:17 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote: > Patch updated with the comment: > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2a64d2d..5fd6a82 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -12455,9 +12455,18 @@ ix86_address_cost (rtx x, enum machine_mode, > addr_space_t, bool) > || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER))) > cost++; > > + /* When address base or index is "pic_offset_table_rtx" we don't increase > + address cost. When a memop with "pic_offset_table_rtx" is not invariant > + itself it most likely means that base or index is not invariant. > + Therefore only "pic_offset_table_rtx" could be hoisted out, which is not > + profitable for x86. */ > if (parts.base > + && (!pic_offset_table_rtx > + || REGNO (pic_offset_table_rtx) != REGNO(parts.base)) > && (!REG_P (parts.base) || REGNO (parts.base) >= FIRST_PSEUDO_REGISTER) > && parts.index > + && (!pic_offset_table_rtx > + || REGNO (pic_offset_table_rtx) != REGNO(parts.index)) > && (!REG_P (parts.index) || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER) > && parts.base != parts.index) > cost++; LGTM. OK. Thanks, Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2a64d2d..5fd6a82 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -12455,9 +12455,18 @@ ix86_address_cost (rtx x, enum machine_mode, addr_space_t, bool) || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER))) cost++; + /* When address base or index is "pic_offset_table_rtx" we don't increase + address cost. When a memop with "pic_offset_table_rtx" is not invariant + itself it most likely means that base or index is not invariant. + Therefore only "pic_offset_table_rtx" could be hoisted out, which is not + profitable for x86. */ if (parts.base + && (!pic_offset_table_rtx + || REGNO (pic_offset_table_rtx) != REGNO(parts.base)) && (!REG_P (parts.base) || REGNO (parts.base) >= FIRST_PSEUDO_REGISTER) && parts.index + && (!pic_offset_table_rtx + || REGNO (pic_offset_table_rtx) != REGNO(parts.index)) && (!REG_P (parts.index) || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER) && parts.base != parts.index)
Patch updated with the comment: cost++; On Fri, Oct 10, 2014 at 3:04 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, Oct 10, 2014 at 9:58 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote: >> the patch improves performance when previous are applied. >> It makes RTL loop invariant behavior for GOT loads same as it was >> before the 2 previous patches. > >> The patch fixes x86 address cost so that cost for addresses with GOT >> register becomes less, how it was before enabling EBX. >> >> In x86_address_cost the result of “REGNO (parts.base) >= >> FIRST_PSEUDO_REGISTER” for hard ebx was always false. The patch makes >> condition result >> the same when parts.base is GOT register (the same for parts.index). >> >> 2014-10-08 Evgeny Stupachenko <evstupac@gmail.com> >> * gcc/config/i386/i386.c (ix86_address_cost): Lower cost for >> when address contains GOT register. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index b43e870..9d8cfd1 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -12497,8 +12497,12 @@ ix86_address_cost (rtx x, enum machine_mode, >> addr_space_t, bool) >> cost++; > > Please add a short comment here, explaining the reason for new condition. > >> if (parts.base >> + && (!pic_offset_table_rtx >> + || REGNO (pic_offset_table_rtx) != REGNO(parts.base)) >> && (!REG_P (parts.base) || REGNO (parts.base) >= FIRST_PSEUDO_REGISTER) >> && parts.index >> + && (!pic_offset_table_rtx >> + || REGNO (pic_offset_table_rtx) != REGNO(parts.index)) >> && (!REG_P (parts.index) || REGNO (parts.index) >= FIRST_PSEUDO_REGISTER) >> && parts.base != parts.index) >> cost++; > > Otherwise LGTM, but please repost the patch with a comment. > > Uros.