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

Reply via email to