| Message ID | CAFULd4aPFSEy+m5mx73JLbmqaNpanfa2ravbCtuUsTNFpkO+Ow@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 27, 2014 at 3:19 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > Hello! > > Attached patch helps RA to choose the most appropriate PIC register by > changing the register preference for set_got patterns. Using this > patch, there should really be a reason for RA to avoid ABI mandated > hard PIC reg. > > This patch avoids many mov %exx,%ebx in front of the calls, that > happen with unpatched compiler even with Vladimir's latest RA patch to > avoid duplicated PIC registers. > > As a smoke test, I have checked 32bit libgo.so.6.0.0 library, where now we have: > > [uros@omen7 .libs]$ grep thunk.bx aaa | wc -l > 7693 > [uros@omen7 .libs]$ grep thunk.ax aaa | wc -l > 10 > [uros@omen7 .libs]$ grep thunk.cx aaa | wc -l > 4 > [uros@omen7 .libs]$ grep thunk.dx aaa | wc -l > 8 > [uros@omen7 .libs]$ grep thunk.bp aaa | wc -l > 497 > [uros@omen7 .libs]$ grep thunk.si aaa | wc -l > 145 > [uros@omen7 .libs]$ grep thunk.di aaa | wc -l > 198 > > 2014-11-27 Uros Bizjak <ubizjak@gmail.com> > > * config/i386/i386.md (set_got): Use "=b,?r" constraint for operand 0. EBX is needed only if PLT is used. If PLT isn't used, we should try caller saved registers first. > (set_got_labelled): Ditto. > (set_got_rex64): Ditto. > (set_rip_rex64): Ditto. > (set_got_offset_rex64): Ditto. We shouldn't do it for 64-bit. > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. > > Thoughts? > > Uros
On 11/27/14 04:19, Uros Bizjak wrote: > Hello! > > Attached patch helps RA to choose the most appropriate PIC register by > changing the register preference for set_got patterns. Using this > patch, there should really be a reason for RA to avoid ABI mandated > hard PIC reg. Agreed. > > This patch avoids many mov %exx,%ebx in front of the calls, that > happen with unpatched compiler even with Vladimir's latest RA patch to > avoid duplicated PIC registers. [ ... ] Far more than I would have expected. > 2014-11-27 Uros Bizjak <ubizjak@gmail.com> > > * config/i386/i386.md (set_got): Use "=b,?r" constraint for operand 0. > (set_got_labelled): Ditto. > (set_got_rex64): Ditto. > (set_rip_rex64): Ditto. > (set_got_offset_rex64): Ditto. > > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. > > Thoughts? As HJ mentioned, it's not perfect, but it's certainly better than what we're doing now. The only thing I don't like is that it's really disguising failings in IRA/LRA. Can you file a bug for that issue so that Vlad can track it. jeff
On Mon, Dec 01, 2014 at 10:47:40AM -0700, Jeff Law wrote: > > * config/i386/i386.md (set_got): Use "=b,?r" constraint for operand 0. > > (set_got_labelled): Ditto. > > (set_got_rex64): Ditto. > > (set_rip_rex64): Ditto. > > (set_got_offset_rex64): Ditto. > > > >Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. > > > >Thoughts? > As HJ mentioned, it's not perfect, but it's certainly better than what we're > doing now. > > The only thing I don't like is that it's really disguising failings in > IRA/LRA. As HJ mentioned, it should be done for 32-bit patterns only, at least for now, there is no class for the r15 register. Jakub
On Mon, Dec 1, 2014 at 6:47 PM, Jeff Law <law@redhat.com> wrote: > On 11/27/14 04:19, Uros Bizjak wrote: >> Attached patch helps RA to choose the most appropriate PIC register by >> changing the register preference for set_got patterns. Using this >> patch, there should really be a reason for RA to avoid ABI mandated >> hard PIC reg. > > Agreed. > >> >> This patch avoids many mov %exx,%ebx in front of the calls, that >> happen with unpatched compiler even with Vladimir's latest RA patch to >> avoid duplicated PIC registers. > > [ ... ] > Far more than I would have expected. > > >> 2014-11-27 Uros Bizjak <ubizjak@gmail.com> >> >> * config/i386/i386.md (set_got): Use "=b,?r" constraint for operand >> 0. >> (set_got_labelled): Ditto. >> (set_got_rex64): Ditto. >> (set_rip_rex64): Ditto. >> (set_got_offset_rex64): Ditto. >> >> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. >> >> Thoughts? > > As HJ mentioned, it's not perfect, but it's certainly better than what we're > doing now. > > The only thing I don't like is that it's really disguising failings in > IRA/LRA. Yes, this is the reason I have second thoughts about the patch. I think that it papers over real issue in RA. > Can you file a bug for that issue so that Vlad can track it. Actually, I noticed it in libgo, where searching for various non-bx thunks reveals many cases. I don't have a small testcase at hand ATM. Uros.
Index: config/i386/i386.md =================================================================== --- config/i386/i386.md (revision 218111) +++ config/i386/i386.md (working copy) @@ -12101,7 +12101,7 @@ "ix86_expand_prologue (); DONE;") (define_insn "set_got" - [(set (match_operand:SI 0 "register_operand" "=r") + [(set (match_operand:SI 0 "register_operand" "=b,?r") (unspec:SI [(const_int 0)] UNSPEC_SET_GOT)) (clobber (reg:CC FLAGS_REG))] "!TARGET_64BIT" @@ -12110,7 +12110,7 @@ (set_attr "length" "12")]) (define_insn "set_got_labelled" - [(set (match_operand:SI 0 "register_operand" "=r") + [(set (match_operand:SI 0 "register_operand" "=b,?r") (unspec:SI [(label_ref (match_operand 1))] UNSPEC_SET_GOT)) (clobber (reg:CC FLAGS_REG))] @@ -12120,7 +12120,7 @@ (set_attr "length" "12")]) (define_insn "set_got_rex64" - [(set (match_operand:DI 0 "register_operand" "=r") + [(set (match_operand:DI 0 "register_operand" "=b,?r") (unspec:DI [(const_int 0)] UNSPEC_SET_GOT))] "TARGET_64BIT" "lea{q}\t{_GLOBAL_OFFSET_TABLE_(%%rip), %0|%0, _GLOBAL_OFFSET_TABLE_[rip]}" @@ -12129,7 +12129,7 @@ (set_attr "mode" "DI")]) (define_insn "set_rip_rex64" - [(set (match_operand:DI 0 "register_operand" "=r") + [(set (match_operand:DI 0 "register_operand" "=b,?r") (unspec:DI [(label_ref (match_operand 1))] UNSPEC_SET_RIP))] "TARGET_64BIT" "lea{q}\t{%l1(%%rip), %0|%0, %l1[rip]}" @@ -12138,7 +12138,7 @@ (set_attr "mode" "DI")]) (define_insn "set_got_offset_rex64" - [(set (match_operand:DI 0 "register_operand" "=r") + [(set (match_operand:DI 0 "register_operand" "=b,?r") (unspec:DI [(label_ref (match_operand 1))] UNSPEC_SET_GOT_OFFSET))]
Hello! Attached patch helps RA to choose the most appropriate PIC register by changing the register preference for set_got patterns. Using this patch, there should really be a reason for RA to avoid ABI mandated hard PIC reg. This patch avoids many mov %exx,%ebx in front of the calls, that happen with unpatched compiler even with Vladimir's latest RA patch to avoid duplicated PIC registers. As a smoke test, I have checked 32bit libgo.so.6.0.0 library, where now we have: [uros@omen7 .libs]$ grep thunk.bx aaa | wc -l 7693 [uros@omen7 .libs]$ grep thunk.ax aaa | wc -l 10 [uros@omen7 .libs]$ grep thunk.cx aaa | wc -l 4 [uros@omen7 .libs]$ grep thunk.dx aaa | wc -l 8 [uros@omen7 .libs]$ grep thunk.bp aaa | wc -l 497 [uros@omen7 .libs]$ grep thunk.si aaa | wc -l 145 [uros@omen7 .libs]$ grep thunk.di aaa | wc -l 198 2014-11-27 Uros Bizjak <ubizjak@gmail.com> * config/i386/i386.md (set_got): Use "=b,?r" constraint for operand 0. (set_got_labelled): Ditto. (set_got_rex64): Ditto. (set_rip_rex64): Ditto. (set_got_offset_rex64): Ditto. Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}. Thoughts? Uros