Hi Jakub

Instead of disabling the sibcall, it could also be a valid tail call
optimization by moving the str after ldmia, and change the used
register(It should be handled by RA automatically), as following

         ...
          add     r4, r1, r4, lsl #2
          ldmia   r2, {r1, r2}
          str     r4, [sp, #48]
          ...

thanks
Carrot

On Mon, Feb 6, 2012 at 9:01 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> The attached testcase is miscompiled on arm*, by doing a sibcall when setup
> of one argument overwrites incoming arguments used to setup parameters in
> later insns.
> The reason why
> mem_overlaps_already_clobbered_arg_p/check_sibcall_argument_overlap
> fails to detect is that the caller has non-zero
> crtl->args.pretend_args_size, and in that case the base:
>      /* The argument block when performing a sibling call is the
>         incoming argument block.  */
>      if (pass == 0)
>        {
>          argblock = crtl->args.internal_arg_pointer;
>          argblock
> #ifdef STACK_GROWS_DOWNWARD
>            = plus_constant (argblock, crtl->args.pretend_args_size);
> #else
>            = plus_constant (argblock, -crtl->args.pretend_args_size);
> #endif
>          stored_args_map = sbitmap_alloc (args_size.constant);
>          sbitmap_zero (stored_args_map);
>        }
> apparently isn't virtual-incoming-rtx, but that plus pretend_args_size
> (8 in this case).  When we store bits into stored_args_map sbitmap,
> we use arg->locate.slot_offset.constant based values (or something different
> for ARGS_GROW_DOWNWARD, but when mem_overlaps_already_clobbered_arg_p is
> testing those bits, it uses just virtual-incoming-rtx offsets (or something
> different for ARGS_GROW_DOWNWARD).  This patch fixes it by adjusting the
> virtual-incoming-rtx relative offset to be actually argblock relative
> offset.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux and tested on the
> testcase on arm cross.  Ok for trunk?
>
> 2012-02-06  Jakub Jelinek  <ja...@redhat.com>
>
>        PR target/52129
>        * calls.c (mem_overlaps_already_clobbered_arg_p): If val is
>        CONST_INT_P, subtract resp. add crtl->args.pretend_args_size to it.
>
>        * gcc.c-torture/execute/pr52129.c: New test.
>
> --- gcc/calls.c.jj      2012-02-01 14:44:27.000000000 +0100
> +++ gcc/calls.c 2012-02-06 10:19:12.112132905 +0100
> @@ -1808,6 +1808,11 @@ mem_overlaps_already_clobbered_arg_p (rt
>     return true;
>   else
>     i = INTVAL (val);
> +#ifdef STACK_GROWS_DOWNWARD
> +  i -= crtl->args.pretend_args_size;
> +#else
> +  i += crtl->args.pretend_args_size;
> +#endif
>
>  #ifdef ARGS_GROW_DOWNWARD
>   i = -i - size;
> --- gcc/testsuite/gcc.c-torture/execute/pr52129.c.jj    2012-02-06 
> 10:27:50.988876791 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr52129.c       2012-02-06 
> 10:25:26.000000000 +0100
> @@ -0,0 +1,28 @@
> +/* PR target/52129 */
> +
> +extern void abort (void);
> +struct S { void *p; unsigned int q; };
> +struct T { char a[64]; char b[64]; } t;
> +
> +__attribute__((noinline, noclone)) int
> +foo (void *x, struct S s, void *y, void *z)
> +{
> +  if (x != &t.a[2] || s.p != &t.b[5] || s.q != 27 || y != &t.a[17] || z != 
> &t.b[17])
> +    abort ();
> +  return 29;
> +}
> +
> +__attribute__((noinline, noclone)) int
> +bar (void *x, void *y, void *z, struct S s, int t, struct T *u)
> +{
> +  return foo (x, s, &u->a[t], &u->b[t]);
> +}
> +
> +int
> +main ()
> +{
> +  struct S s = { &t.b[5], 27 };
> +  if (bar (&t.a[2], (void *) 0, (void *) 0, s, 17, &t) != 29)
> +    abort ();
> +  return 0;
> +}
>
>        Jakub

Reply via email to