public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix for PR64353
@ 2015-01-14 14:40 Ilya Enkovich
  2015-01-14 14:52 ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Enkovich @ 2015-01-14 14:40 UTC (permalink / raw)
  To: gcc-patches

Hi,

SRA gimple passes may add loads to functions with no SSA update.  Later it causes ICE when function with not updated SSA is processed by gimple passes.  This patch fixes it by calling update_ssa.

Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for trunk?

Thanks,
Ilya
--
gcc/

2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* ipa-prop.c (ipa_modify_call_arguments): Update SSA for
	vops after adding a load.


gcc/testsuite/

2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* g++.dg/pr64353.C: New.


diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 01f4111..533dcfe 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -4054,6 +4054,8 @@ ipa_modify_call_arguments (struct cgraph_edge *cs, gcall *stmt,
 		    expr = create_tmp_reg (TREE_TYPE (expr));
 		  gimple_assign_set_lhs (tem, expr);
 		  gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
+		  if (gimple_in_ssa_p (cfun))
+		    update_ssa (TODO_update_ssa_only_virtuals);
 		}
 	    }
 	  else
diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
new file mode 100644
index 0000000..7859918
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr64353.C
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+class C
+{
+  int y, x;
+  void i ();
+  bool __attribute__((const)) xx () { return x; }
+};
+
+void C::i ()
+{
+  if (xx ())
+    x = 1;
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-14 14:40 [PATCH] Fix for PR64353 Ilya Enkovich
@ 2015-01-14 14:52 ` Richard Biener
  2015-01-14 17:11   ` Ilya Enkovich
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2015-01-14 14:52 UTC (permalink / raw)
  To: Ilya Enkovich; +Cc: GCC Patches

On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> SRA gimple passes may add loads to functions with no SSA update.  Later it causes ICE when function with not updated SSA is processed by gimple passes.  This patch fixes it by calling update_ssa.
>
> Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for trunk?

No.  I have removed this quadratic update-ssa call previously.  It should
simply keep SSA for up-to-date manually (see how it does gimple_set_vuse
in some cases, probably not all required ones?).

Richard.

> Thanks,
> Ilya
> --
> gcc/
>
> 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR middle-end/64353
>         * ipa-prop.c (ipa_modify_call_arguments): Update SSA for
>         vops after adding a load.
>
>
> gcc/testsuite/
>
> 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR middle-end/64353
>         * g++.dg/pr64353.C: New.
>
>
> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 01f4111..533dcfe 100644
> --- a/gcc/ipa-prop.c
> +++ b/gcc/ipa-prop.c
> @@ -4054,6 +4054,8 @@ ipa_modify_call_arguments (struct cgraph_edge *cs, gcall *stmt,
>                     expr = create_tmp_reg (TREE_TYPE (expr));
>                   gimple_assign_set_lhs (tem, expr);
>                   gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
> +                 if (gimple_in_ssa_p (cfun))
> +                   update_ssa (TODO_update_ssa_only_virtuals);
>                 }
>             }
>           else
> diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
> new file mode 100644
> index 0000000..7859918
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr64353.C
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +class C
> +{
> +  int y, x;
> +  void i ();
> +  bool __attribute__((const)) xx () { return x; }
> +};
> +
> +void C::i ()
> +{
> +  if (xx ())
> +    x = 1;
> +}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-14 14:52 ` Richard Biener
@ 2015-01-14 17:11   ` Ilya Enkovich
  2015-01-14 19:20     ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Enkovich @ 2015-01-14 17:11 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 14 Jan 15:35, Richard Biener wrote:
> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> > Hi,
> >
> > SRA gimple passes may add loads to functions with no SSA update.  Later it causes ICE when function with not updated SSA is processed by gimple passes.  This patch fixes it by calling update_ssa.
> >
> > Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for trunk?
> 
> No.  I have removed this quadratic update-ssa call previously.  It should
> simply keep SSA for up-to-date manually (see how it does gimple_set_vuse
> in some cases, probably not all required ones?).
> 

Would it be OK to call update_ssa only in case we don't have a proper VUSE for call?  Are we allowed to just emit error due to incorrect attribute?

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 01f4111..4ce7822 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -4054,6 +4054,11 @@ ipa_modify_call_arguments (struct cgraph_edge *cs, gcall *stmt,
 		    expr = create_tmp_reg (TREE_TYPE (expr));
 		  gimple_assign_set_lhs (tem, expr);
 		  gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
+		  /* In case callee has a wrong __attribute__((const))
+		     we may have no VUSE for the call and thus require
+		     SSA update for the inserted load.  See PR64353.  */
+		  if (gimple_in_ssa_p (cfun) && !gimple_vuse (stmt))
+		    update_ssa (TODO_update_ssa_only_virtuals);
 		}
 	    }
 	  else

Thanks,
Ilya

> Richard.
> 
> > Thanks,
> > Ilya
> > --
> > gcc/
> >
> > 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         PR middle-end/64353
> >         * ipa-prop.c (ipa_modify_call_arguments): Update SSA for
> >         vops after adding a load.
> >
> >
> > gcc/testsuite/
> >
> > 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         PR middle-end/64353
> >         * g++.dg/pr64353.C: New.
> >
> >
> > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> > index 01f4111..533dcfe 100644
> > --- a/gcc/ipa-prop.c
> > +++ b/gcc/ipa-prop.c
> > @@ -4054,6 +4054,8 @@ ipa_modify_call_arguments (struct cgraph_edge *cs, gcall *stmt,
> >                     expr = create_tmp_reg (TREE_TYPE (expr));
> >                   gimple_assign_set_lhs (tem, expr);
> >                   gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
> > +                 if (gimple_in_ssa_p (cfun))
> > +                   update_ssa (TODO_update_ssa_only_virtuals);
> >                 }
> >             }
> >           else
> > diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
> > new file mode 100644
> > index 0000000..7859918
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/pr64353.C
> > @@ -0,0 +1,15 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +class C
> > +{
> > +  int y, x;
> > +  void i ();
> > +  bool __attribute__((const)) xx () { return x; }
> > +};
> > +
> > +void C::i ()
> > +{
> > +  if (xx ())
> > +    x = 1;
> > +}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-14 17:11   ` Ilya Enkovich
@ 2015-01-14 19:20     ` Richard Biener
  2015-01-16 10:57       ` Ilya Enkovich
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2015-01-14 19:20 UTC (permalink / raw)
  To: Ilya Enkovich; +Cc: GCC Patches

On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>On 14 Jan 15:35, Richard Biener wrote:
>> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
><enkovich.gnu@gmail.com> wrote:
>> > Hi,
>> >
>> > SRA gimple passes may add loads to functions with no SSA update. 
>Later it causes ICE when function with not updated SSA is processed by
>gimple passes.  This patch fixes it by calling update_ssa.
>> >
>> > Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for
>trunk?
>> 
>> No.  I have removed this quadratic update-ssa call previously.  It
>should
>> simply keep SSA for up-to-date manually (see how it does
>gimple_set_vuse
>> in some cases, probably not all required ones?).
>> 
>
>Would it be OK to call update_ssa only in case we don't have a proper
>VUSE for call? 

No, and most definitely not here.


Are we allowed to just emit error due to incorrect
>attribute?

No, I don't think so either. But we may drop it.

Richard.

>
>diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
>index 01f4111..4ce7822 100644
>--- a/gcc/ipa-prop.c
>+++ b/gcc/ipa-prop.c
>@@ -4054,6 +4054,11 @@ ipa_modify_call_arguments (struct cgraph_edge
>*cs, gcall *stmt,
> 		    expr = create_tmp_reg (TREE_TYPE (expr));
> 		  gimple_assign_set_lhs (tem, expr);
> 		  gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
>+		  /* In case callee has a wrong __attribute__((const))
>+		     we may have no VUSE for the call and thus require
>+		     SSA update for the inserted load.  See PR64353.  */
>+		  if (gimple_in_ssa_p (cfun) && !gimple_vuse (stmt))
>+		    update_ssa (TODO_update_ssa_only_virtuals);
> 		}
> 	    }
> 	  else
>
>Thanks,
>Ilya
>
>> Richard.
>> 
>> > Thanks,
>> > Ilya
>> > --
>> > gcc/
>> >
>> > 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         PR middle-end/64353
>> >         * ipa-prop.c (ipa_modify_call_arguments): Update SSA for
>> >         vops after adding a load.
>> >
>> >
>> > gcc/testsuite/
>> >
>> > 2015-01-14  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         PR middle-end/64353
>> >         * g++.dg/pr64353.C: New.
>> >
>> >
>> > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
>> > index 01f4111..533dcfe 100644
>> > --- a/gcc/ipa-prop.c
>> > +++ b/gcc/ipa-prop.c
>> > @@ -4054,6 +4054,8 @@ ipa_modify_call_arguments (struct cgraph_edge
>*cs, gcall *stmt,
>> >                     expr = create_tmp_reg (TREE_TYPE (expr));
>> >                   gimple_assign_set_lhs (tem, expr);
>> >                   gsi_insert_before (&gsi, tem, GSI_SAME_STMT);
>> > +                 if (gimple_in_ssa_p (cfun))
>> > +                   update_ssa (TODO_update_ssa_only_virtuals);
>> >                 }
>> >             }
>> >           else
>> > diff --git a/gcc/testsuite/g++.dg/pr64353.C
>b/gcc/testsuite/g++.dg/pr64353.C
>> > new file mode 100644
>> > index 0000000..7859918
>> > --- /dev/null
>> > +++ b/gcc/testsuite/g++.dg/pr64353.C
>> > @@ -0,0 +1,15 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-O2" } */
>> > +
>> > +class C
>> > +{
>> > +  int y, x;
>> > +  void i ();
>> > +  bool __attribute__((const)) xx () { return x; }
>> > +};
>> > +
>> > +void C::i ()
>> > +{
>> > +  if (xx ())
>> > +    x = 1;
>> > +}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-14 19:20     ` Richard Biener
@ 2015-01-16 10:57       ` Ilya Enkovich
  2015-01-16 11:49         ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Enkovich @ 2015-01-16 10:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 14 Jan 19:40, Richard Biener wrote:
> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> >On 14 Jan 15:35, Richard Biener wrote:
> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
> ><enkovich.gnu@gmail.com> wrote:
> >> > Hi,
> >> >
> >> > SRA gimple passes may add loads to functions with no SSA update. 
> >Later it causes ICE when function with not updated SSA is processed by
> >gimple passes.  This patch fixes it by calling update_ssa.
> >> >
> >> > Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for
> >trunk?
> >> 
> >> No.  I have removed this quadratic update-ssa call previously.  It
> >should
> >> simply keep SSA for up-to-date manually (see how it does
> >gimple_set_vuse
> >> in some cases, probably not all required ones?).
> >> 
> >
> >Would it be OK to call update_ssa only in case we don't have a proper
> >VUSE for call? 
> 
> No, and most definitely not here.
> 
> 
> Are we allowed to just emit error due to incorrect
> >attribute?
> 
> No, I don't think so either. But we may drop it.
> 
> Richard.
> 

Here is a version with SRA disabled for functions with __attribute__((const)) as you suggested in tracker.  Is it OK?

Bootstrapped and checked on x86_64-unknown-linux-gnu.

Thanks,
Ilya
--
gcc/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* tree-sra.c (ipa_sra_preliminary_function_checks): Reject
	functions with const attribute.

gcc/testsuite/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* g++.dg/pr64353.C: New.


diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
new file mode 100644
index 0000000..7859918
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr64353.C
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+class C
+{
+  int y, x;
+  void i ();
+  bool __attribute__((const)) xx () { return x; }
+};
+
+void C::i ()
+{
+  if (xx ())
+    x = 1;
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index f560fe0..f24ca9f 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -5015,6 +5015,13 @@ ipa_sra_preliminary_function_checks (struct cgraph_node *node)
       return false;
     }
 
+  if (lookup_attribute ("const", DECL_ATTRIBUTES (node->decl)))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Function has const attribute.\n");
+      return false;
+    }
+
   if (!tree_versionable_function_p (node->decl))
     {
       if (dump_file)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-16 10:57       ` Ilya Enkovich
@ 2015-01-16 11:49         ` Richard Biener
  2015-01-16 15:57           ` Ilya Enkovich
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2015-01-16 11:49 UTC (permalink / raw)
  To: Ilya Enkovich; +Cc: GCC Patches

On Fri, Jan 16, 2015 at 11:51 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> On 14 Jan 19:40, Richard Biener wrote:
>> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> >On 14 Jan 15:35, Richard Biener wrote:
>> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
>> ><enkovich.gnu@gmail.com> wrote:
>> >> > Hi,
>> >> >
>> >> > SRA gimple passes may add loads to functions with no SSA update.
>> >Later it causes ICE when function with not updated SSA is processed by
>> >gimple passes.  This patch fixes it by calling update_ssa.
>> >> >
>> >> > Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for
>> >trunk?
>> >>
>> >> No.  I have removed this quadratic update-ssa call previously.  It
>> >should
>> >> simply keep SSA for up-to-date manually (see how it does
>> >gimple_set_vuse
>> >> in some cases, probably not all required ones?).
>> >>
>> >
>> >Would it be OK to call update_ssa only in case we don't have a proper
>> >VUSE for call?
>>
>> No, and most definitely not here.
>>
>>
>> Are we allowed to just emit error due to incorrect
>> >attribute?
>>
>> No, I don't think so either. But we may drop it.
>>
>> Richard.
>>
>
> Here is a version with SRA disabled for functions with __attribute__((const)) as you suggested in tracker.  Is it OK?
>
> Bootstrapped and checked on x86_64-unknown-linux-gnu.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR middle-end/64353
>         * tree-sra.c (ipa_sra_preliminary_function_checks): Reject
>         functions with const attribute.
>
> gcc/testsuite/
>
> 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         PR middle-end/64353
>         * g++.dg/pr64353.C: New.
>
>
> diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
> new file mode 100644
> index 0000000..7859918
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr64353.C
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +class C
> +{
> +  int y, x;
> +  void i ();
> +  bool __attribute__((const)) xx () { return x; }
> +};
> +
> +void C::i ()
> +{
> +  if (xx ())
> +    x = 1;
> +}
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index f560fe0..f24ca9f 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -5015,6 +5015,13 @@ ipa_sra_preliminary_function_checks (struct cgraph_node *node)
>        return false;
>      }
>
> +  if (lookup_attribute ("const", DECL_ATTRIBUTES (node->decl)))

TREE_READONLY (node->decl)

but I'm now not sure that we can really trust this given that only fixup_cfg
will eventually fixup stmts in callers after ipa-pure-const ran.  The above
also doesn't handle "no vops" functions.

I think a better place to check this would be inside
some_callers_have_mismatched_arguments_p where you'd check

  || !gimple_vuse (cs->call_stmt)

of course the reasoning printed is wrong then.

We can fix this by running update-ssa in fixup_cfg as well, and I guess
I like that more.

Index: tree-cfg.c
===================================================================
--- tree-cfg.c  (revision 219714)
+++ tree-cfg.c  (working copy)
@@ -8754,7 +8804,7 @@ const pass_data pass_data_fixup_cfg =
   PROP_cfg, /* properties_required */
   0, /* properties_provided */
   0, /* properties_destroyed */
-  0, /* todo_flags_start */
+  TODO_update_ssa_only_virtuals, /* todo_flags_start */
   0, /* todo_flags_finish */
 };

this variant is pre-approved if it passes bootstrap and regtest.

Thanks,
Richard.

> +    {
> +      if (dump_file)
> +       fprintf (dump_file, "Function has const attribute.\n");
> +      return false;
> +    }
> +
>    if (!tree_versionable_function_p (node->decl))
>      {
>        if (dump_file)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for PR64353
  2015-01-16 11:49         ` Richard Biener
@ 2015-01-16 15:57           ` Ilya Enkovich
  0 siblings, 0 replies; 7+ messages in thread
From: Ilya Enkovich @ 2015-01-16 15:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

On 16 Jan 12:38, Richard Biener wrote:
> On Fri, Jan 16, 2015 at 11:51 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> > On 14 Jan 19:40, Richard Biener wrote:
> >> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> >> >On 14 Jan 15:35, Richard Biener wrote:
> >> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
> >> ><enkovich.gnu@gmail.com> wrote:
> >> >> > Hi,
> >> >> >
> >> >> > SRA gimple passes may add loads to functions with no SSA update.
> >> >Later it causes ICE when function with not updated SSA is processed by
> >> >gimple passes.  This patch fixes it by calling update_ssa.
> >> >> >
> >> >> > Bootstrapped and checked on x86_64-unknown-linux-gnu.  OK for
> >> >trunk?
> >> >>
> >> >> No.  I have removed this quadratic update-ssa call previously.  It
> >> >should
> >> >> simply keep SSA for up-to-date manually (see how it does
> >> >gimple_set_vuse
> >> >> in some cases, probably not all required ones?).
> >> >>
> >> >
> >> >Would it be OK to call update_ssa only in case we don't have a proper
> >> >VUSE for call?
> >>
> >> No, and most definitely not here.
> >>
> >>
> >> Are we allowed to just emit error due to incorrect
> >> >attribute?
> >>
> >> No, I don't think so either. But we may drop it.
> >>
> >> Richard.
> >>
> >
> > Here is a version with SRA disabled for functions with __attribute__((const)) as you suggested in tracker.  Is it OK?
> >
> > Bootstrapped and checked on x86_64-unknown-linux-gnu.
> >
> > Thanks,
> > Ilya
> > --
> > gcc/
> >
> > 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         PR middle-end/64353
> >         * tree-sra.c (ipa_sra_preliminary_function_checks): Reject
> >         functions with const attribute.
> >
> > gcc/testsuite/
> >
> > 2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         PR middle-end/64353
> >         * g++.dg/pr64353.C: New.
> >
> >
> > diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
> > new file mode 100644
> > index 0000000..7859918
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/pr64353.C
> > @@ -0,0 +1,15 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +
> > +class C
> > +{
> > +  int y, x;
> > +  void i ();
> > +  bool __attribute__((const)) xx () { return x; }
> > +};
> > +
> > +void C::i ()
> > +{
> > +  if (xx ())
> > +    x = 1;
> > +}
> > diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> > index f560fe0..f24ca9f 100644
> > --- a/gcc/tree-sra.c
> > +++ b/gcc/tree-sra.c
> > @@ -5015,6 +5015,13 @@ ipa_sra_preliminary_function_checks (struct cgraph_node *node)
> >        return false;
> >      }
> >
> > +  if (lookup_attribute ("const", DECL_ATTRIBUTES (node->decl)))
> 
> TREE_READONLY (node->decl)
> 
> but I'm now not sure that we can really trust this given that only fixup_cfg
> will eventually fixup stmts in callers after ipa-pure-const ran.  The above
> also doesn't handle "no vops" functions.
> 
> I think a better place to check this would be inside
> some_callers_have_mismatched_arguments_p where you'd check
> 
>   || !gimple_vuse (cs->call_stmt)
> 
> of course the reasoning printed is wrong then.
> 
> We can fix this by running update-ssa in fixup_cfg as well, and I guess
> I like that more.
> 
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c  (revision 219714)
> +++ tree-cfg.c  (working copy)
> @@ -8754,7 +8804,7 @@ const pass_data pass_data_fixup_cfg =
>    PROP_cfg, /* properties_required */
>    0, /* properties_provided */
>    0, /* properties_destroyed */
> -  0, /* todo_flags_start */
> +  TODO_update_ssa_only_virtuals, /* todo_flags_start */
>    0, /* todo_flags_finish */
>  };
> 
> this variant is pre-approved if it passes bootstrap and regtest.
> 
> Thanks,
> Richard.
> 

Bootstrap and regtest is OK on x86_64-unknown-linux-gnu.  Here is a committed patch.

Thanks,
Ilya
--
gcc/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* tree-cfg.c (pass_data_fixup_cfg): Update SSA for
	virtuals on start.

gcc/testsuite/

2015-01-16  Ilya Enkovich  <ilya.enkovich@intel.com>

	PR middle-end/64353
	* g++.dg/pr64353.C: New.


diff --git a/gcc/testsuite/g++.dg/pr64353.C b/gcc/testsuite/g++.dg/pr64353.C
new file mode 100644
index 0000000..7859918
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr64353.C
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+class C
+{
+  int y, x;
+  void i ();
+  bool __attribute__((const)) xx () { return x; }
+};
+
+void C::i ()
+{
+  if (xx ())
+    x = 1;
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index a9a2c2f..b56a453 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -8741,7 +8741,7 @@ const pass_data pass_data_fixup_cfg =
   PROP_cfg, /* properties_required */
   0, /* properties_provided */
   0, /* properties_destroyed */
-  0, /* todo_flags_start */
+  TODO_update_ssa_only_virtuals, /* todo_flags_start */
   0, /* todo_flags_finish */
 };
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-01-16 15:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-14 14:40 [PATCH] Fix for PR64353 Ilya Enkovich
2015-01-14 14:52 ` Richard Biener
2015-01-14 17:11   ` Ilya Enkovich
2015-01-14 19:20     ` Richard Biener
2015-01-16 10:57       ` Ilya Enkovich
2015-01-16 11:49         ` Richard Biener
2015-01-16 15:57           ` Ilya Enkovich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).