Re: [PATCH, x86, 63534] Fix '-p' profile for 32 bit PIC mode

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Oct 28, 2014 at 04:10:12PM +0300, Evgeny Stupachenko wrote:
> Thank you, Jakub.
> 
> The following patch passed bootstrap, gcc make check and spec2000 with
> "-p -m32 -fPIC".
> Is it ok?
> 
> ChangeLog:
> 
> 2014-10-28  Evgeny Stupachenko  <evstupac@gmail.com>
> 
> gcc/testsuite
>         * gcc.target/i386/mcount_pic.c: New.
> 
> gcc/

Please mention
	PR target/63534
in the ChangeLog entry.

> @@ -10818,6 +10833,36 @@ ix86_finalize_stack_realign_flags (void)
>    crtl->stack_realign_finalized = true;
>  }
> 
> +/* Delete first SET_GOT allocated to reg.  */
> +
> +static void
> +ix86_elim_set_got (rtx reg)
> +{
> +  basic_block bb;
> +  FOR_EACH_BB_FN (bb, cfun)
> +    {
> +      rtx_insn *c_insn;
> +      FOR_BB_INSNS (bb, c_insn)
> +       {
> +         if (GET_CODE (c_insn) == INSN)
> +           {
> +             rtx pat = PATTERN (c_insn);
> +             if (GET_CODE (pat) == PARALLEL)
> +               {
> +                 rtx vec = XVECEXP (pat, 0, 0);
> +                 if (GET_CODE (vec) == SET
> +                    && XINT (XEXP (vec, 1), 1) == UNSPEC_SET_GOT
> +                    && REGNO (XEXP (vec, 0)) == REGNO (reg))
> +                   {
> +                     delete_insn (c_insn);
> +                     return;
> +                   }
> +               }
> +           }
> +       }
> +    }
> +}
> +

This is unsafe.  What I meant is to just remove set_got insn
that is at the beginning of the first basic block (successor of
entry bb), perhaps after some notes.  You can perhaps generalize
that a little bit and look at other insns in the first bb, as long
as no NONDEBUG_INSN_P insn preceeding satisfy
modified_in_p (reg, c_insn) or reg_referenced_p (reg, PATTERN (c_insn))
or so.  But certainly stop at the end of the first bb or at any
real insn that might clobber/set %ebx or use it.

Also, instead of GET_CODE (c_insn) == INSN please use
NONJUMP_INSN_P (c_insn).

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/mcount_pic.c
> @@ -0,0 +1,12 @@
> +/* Test check correct mcount generation.  */
> +/* { dg-do run } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-O2 -fpic -p -save-temps" } */
> +
> +int main()
> +{
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler "mcount" } } */
> +/* { dg-final { scan-assembler "get_pc_thunk" } } */

Missing cleanup-save-temps.  Is _mcount the name of the profiling routine
on all i?86 targets?

	Jakub