On Wed, Nov 12, 2014 at 1:39 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 01:11:40PM -0800, H.J. Lu wrote:
>> On Wed, Nov 12, 2014 at 1:02 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>> > On Wed, Nov 12, 2014 at 12:43:17PM -0800, H.J. Lu wrote:
>> >> @@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree,
>> >> HOST_WIDE_INT delta,
>> >> else
>> >> {
>> >> if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
>> >> - fnaddr = legitimize_pic_address (fnaddr,
>> >> - gen_rtx_REG (Pmode, tmp_regno));
>> >> + {
>> >> + SET_REGNO (pic_offset_table_rtx, R11_REG);
>> >
>> > If pic_offset_table_rtx has never been initialized, how you can use
>>
>> It is a pseudo PIC register which is uninitialized:
>>
>> (gdb) call debug_rtx (this_target_rtl->x_pic_offset_table_rtx)
>> (reg:DI 89)
>> (gdb)
>>
>> > SET_REGNO on it? Shouldn't that be pic_offset_table_rtx = gen_raw_REG
>> > (Pmode, R11_REG);
>>
>> I added the following comments and am checking it into trunk:
>>
>> // CM_LARGE_PIC always uses pseudo PIC register which is
>> // uninitialized. Since FUNCTION is local and calling it
>> // doesn't go through PLT, we use scratch register %r11 as
>> // PIC register and initialize it here.
>
> From what I can see, it probably in this case should be always non-NULL,
> as it is initialized in init_emit_regs:
> 5847 if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM)
> 5848 pic_offset_table_rtx = gen_raw_REG (Pmode,
> PIC_OFFSET_TABLE_REGNUM);
> 5849 else
> 5850 pic_offset_table_rtx = NULL_RTX;
> and as pic_offset_table_rtx is NULL there, PIC_OFFSET_TABLE_REGNUM is
> BX_REG.
> Later on, assign_params will change that to:
> 3684 if (targetm.use_pseudo_pic_reg ())
> 3685 pic_offset_table_rtx = gen_reg_rtx (Pmode);
> and do_reload, after reload temporarily changing the REGNO, will overwrite
> it again to the chosen pseudo:
> 5470 if (pic_offset_table_regno != INVALID_REGNUM)
> 5471 pic_offset_table_rtx = gen_rtx_REG (Pmode,
> pic_offset_table_regno);
>
> So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
> gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
> same.
>
> Jakub
It makes sense. I checked in the following patch.
Thanks.
--
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 217445)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
2014-11-12 H.J. Lu <hongjiu...@intel.com>
+ * config/i386/i386.c (x86_output_mi_thunk): Set gen_rtx_REG to
+ set pic_offset_table_rtx.
+
+2014-11-12 H.J. Lu <hongjiu...@intel.com>
+
PR target/63815
* config/i386/i386.c (ix86_init_large_pic_reg): New. Extracted
from ...
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 217445)
+++ config/i386/i386.c (working copy)
@@ -42697,7 +42697,7 @@ x86_output_mi_thunk (FILE *file, tree, H
// uninitialized. Since FUNCTION is local and calling it
// doesn't go through PLT, we use scratch register %r11 as
// PIC register and initialize it here.
- SET_REGNO (pic_offset_table_rtx, R11_REG);
+ pic_offset_table_rtx = gen_rtx_REG (Pmode, R11_REG);
ix86_init_large_pic_reg (tmp_regno);
fnaddr = legitimize_pic_address (fnaddr,
gen_rtx_REG (Pmode, tmp_regno));
