Re: [PATCH] Fix DCE from removing instructions setting hard frame pointer or other artificially used registers (PR middle-end/32758)

["Seongbae Park (박성배, 朴成培)" <seongbae dot park at gmail dot com>]

Hi Jakub,

Can you show me an example of the bad DCE your change prevents ?
On the face of it, the change looks wrong,
as the artificial uses for a block aren't meant to be alive everywhere.

Beside, it's a per-block set, so the patch as it is
(even if making them live everywhere is what we want to do)
is fragile - it should really use df_get_artificial_uses(bb_index)
to initialize a temporary bitmap.

Seongbae

On 8/29/07, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Paolo asked me to mail this version of the patch, more variants are
> discussed in the PR.
> Bootstrapped/regtested on x86_64-linux and regtested on ppc-linux
> (on the latter with
> -FAIL: noclass execution - gij test
> -FAIL: pr11951 execution - gij test
> -FAIL: throwit execution - gij test
> -FAIL: pr29812 execution - gij test
> -FAIL: ExtraClassLoader execution - source compiled test
> -FAIL: ExtraClassLoader -findirect-dispatch execution - source compiled test
> -FAIL: ExtraClassLoader -O3 execution - source compiled test
> -FAIL: ExtraClassLoader -O3 -findirect-dispatch execution - source compiled test
> effect).  Ok for trunk?
>
> 2007-08-29  Jakub Jelinek  <jakub@redhat.com>
>
>         PR middle-end/32758
>         * dce.c (dce_process_block): Don't delete setters of
>         artificially used registers.
>
>         * gcc.dg/cleanup-12.c: New test.
>
> --- gcc/dce.c.jj        2007-08-13 15:11:18.000000000 +0200
> +++ gcc/dce.c   2007-08-29 16:34:00.000000000 +0200
> @@ -527,6 +527,7 @@ static bool
>  dce_process_block (basic_block bb, bool redo_out)
>  {
>    bitmap local_live = BITMAP_ALLOC (&dce_tmp_bitmap_obstack);
> +  bitmap au;
>    rtx insn;
>    bool block_changed;
>    struct df_ref **def_rec, **use_rec;
> @@ -569,6 +570,15 @@ dce_process_block (basic_block bb, bool
>         bitmap_set_bit (local_live, DF_REF_REGNO (use));
>      }
>
> +  /* These regs are considered always live so if they end up dying
> +     because of some def, we need to bring the back again.
> +     Calling df_simulate_fixup_sets has the disadvantage of calling
> +     df_has_eh_preds once per insn, so we cache the information here.  */
> +  if (df_has_eh_preds (bb))
> +    au = df->eh_block_artificial_uses;
> +  else
> +    au = df->regular_block_artificial_uses;
> +
>    FOR_BB_INSNS_REVERSE (bb, insn)
>      if (INSN_P (insn))
>        {
> @@ -580,7 +590,8 @@ dce_process_block (basic_block bb, bool
>
>             /* The insn is needed if there is someone who uses the output.  */
>             for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
> -             if (bitmap_bit_p (local_live, DF_REF_REGNO (*def_rec)))
> +             if (bitmap_bit_p (local_live, DF_REF_REGNO (*def_rec))
> +                 || bitmap_bit_p (au, DF_REF_REGNO (*def_rec)))
>                 {
>                   needed = true;
>                   break;
> --- gcc/testsuite/gcc.dg/cleanup-12.c.jj        2007-08-29 13:48:19.000000000 +0200
> +++ gcc/testsuite/gcc.dg/cleanup-12.c   2007-08-29 13:48:19.000000000 +0200
> @@ -0,0 +1,69 @@
> +/* PR middle-end/32758 */
> +/* HP-UX libunwind.so doesn't provide _UA_END_OF_STACK */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fexceptions" } */
> +/* { dg-skip-if "" { "ia64-*-hpux11.*" }  { "*" } { "" } } */
> +/* Verify unwind info in presence of alloca.  */
> +
> +#include <unwind.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +static _Unwind_Reason_Code
> +force_unwind_stop (int version, _Unwind_Action actions,
> +                  _Unwind_Exception_Class exc_class,
> +                  struct _Unwind_Exception *exc_obj,
> +                  struct _Unwind_Context *context,
> +                  void *stop_parameter)
> +{
> +  if (actions & _UA_END_OF_STACK)
> +    abort ();
> +  return _URC_NO_REASON;
> +}
> +
> +static void force_unwind (void)
> +{
> +  struct _Unwind_Exception *exc = malloc (sizeof (*exc));
> +  memset (&exc->exception_class, 0, sizeof (exc->exception_class));
> +  exc->exception_cleanup = 0;
> +
> +#ifndef __USING_SJLJ_EXCEPTIONS__
> +  _Unwind_ForcedUnwind (exc, force_unwind_stop, 0);
> +#else
> +  _Unwind_SjLj_ForcedUnwind (exc, force_unwind_stop, 0);
> +#endif
> +
> +  abort ();
> +}
> +
> +__attribute__((noinline))
> +void foo (void *x __attribute__((unused)))
> +{
> +  force_unwind ();
> +}
> +
> +__attribute__((noinline))
> +int bar (unsigned int x)
> +{
> +  void *y = __builtin_alloca (x);
> +  foo (y);
> +  return 1;
> +}
> +
> +static void handler (void *p __attribute__((unused)))
> +{
> +  exit (0);
> +}
> +
> +__attribute__((noinline))
> +static void doit ()
> +{
> +  char dummy __attribute__((cleanup (handler)));
> +  bar (1024);
> +}
> +
> +int main ()
> +{
> +  doit ();
> +  abort ();
> +}
>
>         Jakub
-- 
#pragma ident "Seongbae Park, compiler, http://seongbae.blogspot.com";