diff mbox

Fix for PR64353

Message ID 20150114161258.GB56209@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Jan. 14, 2015, 4:23 p.m. UTC
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?


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;
> > +}

Comments

Richard Biener Jan. 14, 2015, 6:40 p.m. UTC | #1
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;
>> > +}
diff mbox

Patch

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