Re: [PATCH, PR tree-optimization/65002] Disable SRA for functions wrongly marked as read-only

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Feb 12, 2015 at 3:14 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> PR65002 is the second issue caused by SRA for functions wrongly marked as read-only.  Previous fix for PR64353 doesn't work for this case because SSA update happens too late.  Here is a patch to disable SRA for such functions to avoid inconsistent SSA issues.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?

Ok.  I'm not entirely happy with this but I guess any other solution has to wait
for GCC 6.

Thanks,
Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-02-12  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR tree-optimization/65002
>         * tree-cfg.c (pass_data_fixup_cfg): Don't update
>         SSA on start.
>         * tree-sra.c (some_callers_have_no_vuse_p): New.
>         (ipa_early_sra): Reject functions whose callers
>         assume funciton is read only.
>
> gcc/testsuite/
>
> 2015-02-12  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR tree-optimization/65002
>         * gcc.dg/pr65002.C: New.
>
>
> diff --git a/gcc/testsuite/gcc.dg/pr65002.C b/gcc/testsuite/gcc.dg/pr65002.C
> new file mode 100644
> index 0000000..ac7c66b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr65002.C
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/65002 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +namespace fastmath {
> +  template <typename T> float floor(const T &) __attribute__((const));
> +  template <typename T> float floor(const T &p1) { return p1; }
> +}
> +using fastmath::floor;
> +class A {
> +public:
> +  A(int, int);
> +  virtual int m_fn1(float) const;
> +};
> +class B : A {
> +public:
> +  B(int, int p2) : A(entity, p2) {}
> +  int m_fn1(float p1) const { long b(floor(p1)); }
> +  int entity;
> +};
> +
> +int a;
> +void Convert() {
> +  if (int *c = 0)
> +    B(*c, a);
> +}
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 2e23553..006bc08 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -8754,7 +8754,7 @@ const pass_data pass_data_fixup_cfg =
>    PROP_cfg, /* properties_required */
>    0, /* properties_provided */
>    0, /* properties_destroyed */
> -  TODO_update_ssa_only_virtuals, /* todo_flags_start */
> +  0, /* todo_flags_start */
>    0, /* todo_flags_finish */
>  };
>
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index ad9584e..7f78e68 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -4890,6 +4890,20 @@ some_callers_have_mismatched_arguments_p (struct cgraph_node *node,
>    return false;
>  }
>
> +/* Return false if all callers have vuse attached to a call statement.  */
> +
> +static bool
> +some_callers_have_no_vuse_p (struct cgraph_node *node,
> +                            void *data ATTRIBUTE_UNUSED)
> +{
> +  struct cgraph_edge *cs;
> +  for (cs = node->callers; cs; cs = cs->next_caller)
> +    if (!cs->call_stmt || !gimple_vuse (cs->call_stmt))
> +      return true;
> +
> +  return false;
> +}
> +
>  /* Convert all callers of NODE.  */
>
>  static bool
> @@ -5116,6 +5130,15 @@ ipa_early_sra (void)
>        goto simple_out;
>      }
>
> +  if (node->call_for_symbol_thunks_and_aliases
> +       (some_callers_have_no_vuse_p, NULL, true))
> +    {
> +      if (dump_file)
> +       fprintf (dump_file, "There are callers with no VUSE attached "
> +                "to a call stmt.\n");
> +      goto simple_out;
> +    }
> +
>    bb_dereferences = XCNEWVEC (HOST_WIDE_INT,
>                                  func_param_count
>                                  * last_basic_block_for_fn (cfun));