On 16 Jan 12:38, Richard Biener wrote:
> On Fri, Jan 16, 2015 at 11:51 AM, Ilya Enkovich <enkovich....@gmail.com>
> wrote:
> > On 14 Jan 19:40, Richard Biener wrote:
> >> On January 14, 2015 5:23:21 PM CET, Ilya Enkovich <enkovich....@gmail.com>
> >> wrote:
> >> >On 14 Jan 15:35, Richard Biener wrote:
> >> >> On Wed, Jan 14, 2015 at 3:28 PM, Ilya Enkovich
> >> ><enkovich....@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.enkov...@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.enkov...@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.enkov...@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.enkov...@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 */
};
